[PATCH] arm64: Refactor conditional logic

Hardevsinh Palaniya posted 1 patch 1 week, 5 days ago
There is a newer version of this series
arch/arm64/kernel/cpufeature.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] arm64: Refactor conditional logic
Posted by Hardevsinh Palaniya 1 week, 5 days ago
Unnecessarily checks ftr_ovr == tmp in an extra else if, which is not
needed because that condition would already be true by default if the
previous conditions are not satisfied.

Signed-off-by: Hardevsinh Palaniya <hardevsinh.palaniya@siliconsignals.io>
---
 arch/arm64/kernel/cpufeature.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 718728a85430..d9021b1b9cff 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -989,7 +989,7 @@ static void init_cpu_ftr_reg(u32 sys_reg, u64 new)
 				/* Override was valid */
 				ftr_new = tmp;
 				str = "forced";
-			} else if (ftr_ovr == tmp) {
+			} else {
 				/* Override was the safe value */
 				str = "already set";
 			}
-- 
2.43.0
Re: [PATCH] arm64: Refactor conditional logic
Posted by Mark Brown 1 week, 5 days ago
On Mon, Nov 11, 2024 at 07:51:45PM +0530, Hardevsinh Palaniya wrote:
> Unnecessarily checks ftr_ovr == tmp in an extra else if, which is not
> needed because that condition would already be true by default if the
> previous conditions are not satisfied.

> @@ -989,7 +989,7 @@ static void init_cpu_ftr_reg(u32 sys_reg, u64 new)
>  				/* Override was valid */
>  				ftr_new = tmp;
>  				str = "forced";
> -			} else if (ftr_ovr == tmp) {
> +			} else {
>  				/* Override was the safe value */
>  				str = "already set";
>  			}

Your changelog wasn't very clear on this but the tests in this if/else
tree are

			if (ftr_ovr != tmp) {
			} else if (ftr_new != tmp) {
			} else if (ftr_ovr == tmp) {
			}

so your analysis is accurate, the first and last tests are the inverse
of each other so onr must be true.  This should be clear from your
commit log.  Also all of those branches set "str" and we then
immediately test

			if (str)

before logging a diagnostic.  If we're looking to reduce unneeded tests
then either that one is redundant too or there's another bug in the
logic (I think from a quick scan just the former).