powerpc: Mark variable `cpumsr` as unused
PATCH 1/1 powerpc: Mark variable `cpumsr` as unused
Add gcc attribute unused for cpumsr variable.
Fix warnings treated as errors with W=1:
arch/powerpc/kernel/process.c:231:16: error: variable ‘cpumsr’ set but not used [-Werror=unused-but-set-variable] arch/powerpc/kernel/process.c:296:16: error: variable ‘cpumsr’ set but not used [-Werror=unused-but-set-variable]
--- arch/powerpc/kernel/process.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
arch/powerpc/kernel/process.c +2 -2
@@ -228,7 +228,7 @@ EXPORT_SYMBOL_GPL(flush_fp_to_thread);void enable_kernel_fp (void ) { -unsigned long cpumsr; +unsigned long cpumsr__maybe_unused ;WARN_ON (preemptible ());
@@ -293,7 +293,7 @@ EXPORT_SYMBOL(giveup_altivec);void enable_kernel_altivec (void ) { -unsigned long cpumsr; +unsigned long cpumsr__maybe_unused ;WARN_ON (preemptible ());
Signature · 1 line
2.11.0
Quoted 8 lines · Mathieu Malaterre
Add gcc attribute unused for
cpumsrvariable.Fix warnings treated as errors with W=1:
arch/powerpc/kernel/process.c:231:16: error: variable ‘cpumsr’ set but not used [-Werror=unused-but-set-variable]arch/powerpc/kernel/process.c:296:16: error: variable ‘cpumsr’ set but not used [-Werror=unused-but-set-variable]Signed-off-by Mathieu Malaterre <[email protected]>
I don't think this is the good way to fix that. This problem was introduced by commit 5c784c8414fb ("powerpc/tm: Remove msr_tm_active()"). That commit should be reverted and fixed.
That commit should have removed the macro and kept the inline function.
Christophe
Quoted 27 lines
--- arch/powerpc/kernel/process.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)arch/powerpc/kernel/process.c +2
-2@@ -228,7 +228,7 @@ EXPORT_SYMBOL_GPL(flush_fp_to_thread);void enable_kernel_fp (void ) { -unsigned long cpumsr; +unsigned long cpumsr__maybe_unused ;WARN_ON (preemptible ());@@ -293,7 +293,7 @@ EXPORT_SYMBOL(giveup_altivec);void enable_kernel_altivec (void ) { -unsigned long cpumsr; +unsigned long cpumsr__maybe_unused ;WARN_ON (preemptible ());
Quoted 8 lines · Christophe Leroy
Quoted 4 lines · Mathieu Malaterre
Add gcc attribute unused for
cpumsrvariable.Fix warnings treated as errors with W=1:
Quoted 8 lines
Quoted 4 lines
arch/powerpc/kernel/process.c:231:16: error: variable ‘cpumsr’ set but not used [-Werror=unused-but-set-variable]arch/powerpc/kernel/process.c:296:16: error: variable ‘cpumsr’ set but not used [-Werror=unused-but-set-variable]Signed-off-by Mathieu Malaterre <[email protected]>I don't think this is the good way to fix that. This problem was introduced by commit
5c784c8414fb("powerpc/tm: Removemsr_tm_active()"). That commit should be reverted and fixed.
I see, it makes sense.
Quoted 1 line
That commit should have removed the macro and kept the inline function.
Breno, what do you think ?
Quoted 29 lines
Christophe
Quoted 27 lines
--- arch/powerpc/kernel/process.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)arch/powerpc/kernel/process.c +2
-2@@ -228,7 +228,7 @@ EXPORT_SYMBOL_GPL(flush_fp_to_thread);void enable_kernel_fp (void ) { -unsigned long cpumsr; +unsigned long cpumsr__maybe_unused ;WARN_ON (preemptible ());@@ -293,7 +293,7 @@ EXPORT_SYMBOL(giveup_altivec);void enable_kernel_altivec (void ) { -unsigned long cpumsr; +unsigned long cpumsr__maybe_unused ;WARN_ON (preemptible ());
Hi Mathieu, Christophe
Thanks for spotting and fixing this bug.
Quoted 18 lines · Mathieu Malaterre
Quoted 8 lines · Christophe Leroy
Quoted 4 lines · Mathieu Malaterre
Add gcc attribute unused for
cpumsrvariable.Fix warnings treated as errors with W=1:
Quoted 8 lines
Quoted 4 lines
arch/powerpc/kernel/process.c:231:16: error: variable ‘cpumsr’ set but not used [-Werror=unused-but-set-variable]arch/powerpc/kernel/process.c:296:16: error: variable ‘cpumsr’ set but not used [-Werror=unused-but-set-variable]Signed-off-by Mathieu Malaterre <[email protected]>I don't think this is the good way to fix that. This problem was introduced by commit
5c784c8414fb("powerpc/tm: Removemsr_tm_active()"). That commit should be reverted and fixed.
Quoted 5 lines
I see, it makes sense.
Quoted 1 line
That commit should have removed the macro and kept the inline function.
Breno, what do you think ?
Turning this macro into a function might cause the code to be more confused, since all the other TM states bits are checked using a macro, for example:
MSR_TM_SUSPENDED Checks if the MSR has Suspended bits set
MSR_TM_TRANSACTIONAL Checks if the MSR has the transactional bits set
MSR_TM_RESV Checks if the MSR has the TM reserved bits set
That said, I understand that it makes sense to have an uniform way to check for TM bits in MSR, thus having a MSR_TM_ACTIVE macro to check for the active bits. Using a non-uniform function just to fix this warning seems to be an overkill. Reverting the patch seems to bring back the old style, which is having a macro and a function with the same name, where the function just calls the macro.
Anyway, I think it might have other ways to fix warning, as I can think now:
1) Avoid setting cpumsr if CONFIG_PPC_TRANSACTIONAL_MEM is not enabled
2) If !CONFIG_PPC_TRANSACTIONAL_MEM, redefine MSR_TM_ACTIVE(x) to something
as (x & 0) instead of 0.
3) Avoid double definition of MSR_TM_ACTIVE, i.e, have the same definition
independent of PPC_TRANSACTIONAL_MEM being set or not.
Anyway, I would like to try option 3), which is the hardest one to implement and validate, but it seems to be the most correct option, once it checks for a MSR bit configuration, and the caller should have the logic.