* [PATCH linux-next] power:pkeys: fix bugon.cocci warnings
@ 2021-08-25 6:42 CGEL
2021-08-31 6:01 ` Daniel Axtens
2022-05-02 13:24 ` Michael Ellerman
0 siblings, 2 replies; 5+ messages in thread
From: CGEL @ 2021-08-25 6:42 UTC (permalink / raw)
To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Sandipan Das, linuxppc-dev, linux-kernel
Cc: Jing Yangyang, Zeal Robot
From: Jing Yangyang <jing.yangyang@zte.com.cn>
Use BUG_ON instead of a if condition followed by BUG.
./arch/powerpc/include/asm/book3s/64/pkeys.h:21:2-5:WARNING
Use BUG_ON instead of if condition followed by BUG.
./arch/powerpc/include/asm/book3s/64/pkeys.h:14:2-5:WARNING
Use BUG_ON instead of if condition followed by BUG.
Generated by: scripts/coccinelle/misc/bugon.cocci
Reported-by: Zeal Robot <zealci@zte.com.cn>
Signed-off-by: Jing Yangyang <jing.yangyang@zte.com.cn>
---
arch/powerpc/include/asm/book3s/64/pkeys.h | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/64/pkeys.h b/arch/powerpc/include/asm/book3s/64/pkeys.h
index 5b17813..5f74f0c 100644
--- a/arch/powerpc/include/asm/book3s/64/pkeys.h
+++ b/arch/powerpc/include/asm/book3s/64/pkeys.h
@@ -10,15 +10,13 @@ static inline u64 vmflag_to_pte_pkey_bits(u64 vm_flags)
if (!mmu_has_feature(MMU_FTR_PKEY))
return 0x0UL;
- if (radix_enabled())
- BUG();
+ BUG_ON(radix_enabled());
return hash__vmflag_to_pte_pkey_bits(vm_flags);
}
static inline u16 pte_to_pkey_bits(u64 pteflags)
{
- if (radix_enabled())
- BUG();
+ BUG_ON(radix_enabled());
return hash__pte_to_pkey_bits(pteflags);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH linux-next] power:pkeys: fix bugon.cocci warnings
2021-08-25 6:42 [PATCH linux-next] power:pkeys: fix bugon.cocci warnings CGEL
@ 2021-08-31 6:01 ` Daniel Axtens
2022-05-02 13:24 ` Michael Ellerman
1 sibling, 0 replies; 5+ messages in thread
From: Daniel Axtens @ 2021-08-31 6:01 UTC (permalink / raw)
To: CGEL, Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
Sandipan Das, linuxppc-dev, linux-kernel
Cc: Zeal Robot, Jing Yangyang
Hi Jing,
Thanks for your patch.
The patch looks good, but looking at the output of `make coccicheck
M=arch/powerpc MODE=report`, it looks like there might be a few other
things that we might want to fix. Would it be worth trying to make the
arch/powerpc directory free from coccinelle warnings in one big patch
series, and then we could add coccicheck to our automatic patch testing?
(see
e.g. https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20210825064228.70487-1-deng.changcheng@zte.com.cn/ )
For this patch, I think we should try to fix all of arch/powerpc at the
same time. The check points out the following other possible uses of
BUG_ON:
arch/powerpc/include/asm/book3s/64/pgtable-64k.h:68:2-5: WARNING: Use BUG_ON instead of if condition followed by BUG.
Please make sure the condition has no side effects (see conditional BUG_ON definition in include/asm-generic/bug.h)
arch/powerpc/platforms/cell/spufs/sched.c:908:2-5: WARNING: Use BUG_ON instead of if condition followed by BUG.
Please make sure the condition has no side effects (see conditional BUG_ON definition in include/asm-generic/bug.h)
arch/powerpc/platforms/powernv/idle.c:968:3-6: WARNING: Use BUG_ON instead of if condition followed by BUG.
Please make sure the condition has no side effects (see conditional BUG_ON definition in include/asm-generic/bug.h)
arch/powerpc/platforms/powernv/idle.c:456:2-5: WARNING: Use BUG_ON instead of if condition followed by BUG.
Please make sure the condition has no side effects (see conditional BUG_ON definition in include/asm-generic/bug.h)
Kind regards,
Daniel
> Use BUG_ON instead of a if condition followed by BUG.
>
> ./arch/powerpc/include/asm/book3s/64/pkeys.h:21:2-5:WARNING
> Use BUG_ON instead of if condition followed by BUG.
> ./arch/powerpc/include/asm/book3s/64/pkeys.h:14:2-5:WARNING
> Use BUG_ON instead of if condition followed by BUG.
>
> Generated by: scripts/coccinelle/misc/bugon.cocci
>
> Reported-by: Zeal Robot <zealci@zte.com.cn>
> Signed-off-by: Jing Yangyang <jing.yangyang@zte.com.cn>
> ---
> arch/powerpc/include/asm/book3s/64/pkeys.h | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/64/pkeys.h b/arch/powerpc/include/asm/book3s/64/pkeys.h
> index 5b17813..5f74f0c 100644
> --- a/arch/powerpc/include/asm/book3s/64/pkeys.h
> +++ b/arch/powerpc/include/asm/book3s/64/pkeys.h
> @@ -10,15 +10,13 @@ static inline u64 vmflag_to_pte_pkey_bits(u64 vm_flags)
> if (!mmu_has_feature(MMU_FTR_PKEY))
> return 0x0UL;
>
> - if (radix_enabled())
> - BUG();
> + BUG_ON(radix_enabled());
> return hash__vmflag_to_pte_pkey_bits(vm_flags);
> }
>
> static inline u16 pte_to_pkey_bits(u64 pteflags)
> {
> - if (radix_enabled())
> - BUG();
> + BUG_ON(radix_enabled());
> return hash__pte_to_pkey_bits(pteflags);
> }
>
> --
> 1.8.3.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH linux-next] power:pkeys: fix bugon.cocci warnings
2021-08-25 6:42 [PATCH linux-next] power:pkeys: fix bugon.cocci warnings CGEL
2021-08-31 6:01 ` Daniel Axtens
@ 2022-05-02 13:24 ` Michael Ellerman
2022-05-07 7:04 ` Christophe Leroy
1 sibling, 1 reply; 5+ messages in thread
From: Michael Ellerman @ 2022-05-02 13:24 UTC (permalink / raw)
To: CGEL, Benjamin Herrenschmidt, Paul Mackerras, Sandipan Das,
linuxppc-dev, linux-kernel
Cc: Jing Yangyang, Zeal Robot
CGEL <cgel.zte@gmail.com> writes:
> From: Jing Yangyang <jing.yangyang@zte.com.cn>
>
> Use BUG_ON instead of a if condition followed by BUG.
>
> ./arch/powerpc/include/asm/book3s/64/pkeys.h:21:2-5:WARNING
> Use BUG_ON instead of if condition followed by BUG.
> ./arch/powerpc/include/asm/book3s/64/pkeys.h:14:2-5:WARNING
> Use BUG_ON instead of if condition followed by BUG.
>
> Generated by: scripts/coccinelle/misc/bugon.cocci
>
> Reported-by: Zeal Robot <zealci@zte.com.cn>
> Signed-off-by: Jing Yangyang <jing.yangyang@zte.com.cn>
> ---
> arch/powerpc/include/asm/book3s/64/pkeys.h | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/book3s/64/pkeys.h b/arch/powerpc/include/asm/book3s/64/pkeys.h
> index 5b17813..5f74f0c 100644
> --- a/arch/powerpc/include/asm/book3s/64/pkeys.h
> +++ b/arch/powerpc/include/asm/book3s/64/pkeys.h
> @@ -10,15 +10,13 @@ static inline u64 vmflag_to_pte_pkey_bits(u64 vm_flags)
> if (!mmu_has_feature(MMU_FTR_PKEY))
> return 0x0UL;
>
> - if (radix_enabled())
> - BUG();
> + BUG_ON(radix_enabled());
> return hash__vmflag_to_pte_pkey_bits(vm_flags);
> }
>
> static inline u16 pte_to_pkey_bits(u64 pteflags)
> {
> - if (radix_enabled())
> - BUG();
> + BUG_ON(radix_enabled());
> return hash__pte_to_pkey_bits(pteflags);
> }
Have you checked how this changes the generated code?
radix_enabled() is a jump label, via mmu_feature().
Possibly the compiler just works it all out and generates the same code,
but I'd want some evidence of that before merging this.
cheers
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH linux-next] power:pkeys: fix bugon.cocci warnings
2022-05-02 13:24 ` Michael Ellerman
@ 2022-05-07 7:04 ` Christophe Leroy
2022-05-07 8:49 ` Arnd Bergmann
0 siblings, 1 reply; 5+ messages in thread
From: Christophe Leroy @ 2022-05-07 7:04 UTC (permalink / raw)
To: Michael Ellerman, CGEL, Benjamin Herrenschmidt, Paul Mackerras,
Sandipan Das, linuxppc-dev, linux-kernel
Cc: Zeal Robot, Jing Yangyang
Le 02/05/2022 à 15:24, Michael Ellerman a écrit :
> CGEL <cgel.zte@gmail.com> writes:
>> From: Jing Yangyang <jing.yangyang@zte.com.cn>
>>
>> Use BUG_ON instead of a if condition followed by BUG.
>>
>> ./arch/powerpc/include/asm/book3s/64/pkeys.h:21:2-5:WARNING
>> Use BUG_ON instead of if condition followed by BUG.
>> ./arch/powerpc/include/asm/book3s/64/pkeys.h:14:2-5:WARNING
>> Use BUG_ON instead of if condition followed by BUG.
>>
>> Generated by: scripts/coccinelle/misc/bugon.cocci
>>
>> Reported-by: Zeal Robot <zealci@zte.com.cn>
>> Signed-off-by: Jing Yangyang <jing.yangyang@zte.com.cn>
>> ---
>> arch/powerpc/include/asm/book3s/64/pkeys.h | 6 ++----
>> 1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/book3s/64/pkeys.h b/arch/powerpc/include/asm/book3s/64/pkeys.h
>> index 5b17813..5f74f0c 100644
>> --- a/arch/powerpc/include/asm/book3s/64/pkeys.h
>> +++ b/arch/powerpc/include/asm/book3s/64/pkeys.h
>> @@ -10,15 +10,13 @@ static inline u64 vmflag_to_pte_pkey_bits(u64 vm_flags)
>> if (!mmu_has_feature(MMU_FTR_PKEY))
>> return 0x0UL;
>>
>> - if (radix_enabled())
>> - BUG();
>> + BUG_ON(radix_enabled());
>> return hash__vmflag_to_pte_pkey_bits(vm_flags);
>> }
>>
>> static inline u16 pte_to_pkey_bits(u64 pteflags)
>> {
>> - if (radix_enabled())
>> - BUG();
>> + BUG_ON(radix_enabled());
>> return hash__pte_to_pkey_bits(pteflags);
>> }
>
> Have you checked how this changes the generated code?
>
> radix_enabled() is a jump label, via mmu_feature().
>
> Possibly the compiler just works it all out and generates the same code,
> but I'd want some evidence of that before merging this.
Seems like the compiler is not that good, the generated code for test1()
is much better than the one for test2(), see below.
void test1(void)
{
if (radix_enabled())
BUG();
}
void test2(void)
{
BUG_ON(radix_enabled());
}
0000000000000900 <.test1>:
900: 60 00 00 00 nop
904: 0f e0 00 00 twui r0,0
908: 60 00 00 00 nop
90c: 60 00 00 00 nop
910: 4e 80 00 20 blr
914: 60 00 00 00 nop
918: 60 00 00 00 nop
91c: 60 00 00 00 nop
0000000000000920 <.test2>:
920: 60 00 00 00 nop
924: 39 20 00 01 li r9,1
928: 0b 09 00 00 tdnei r9,0
92c: 4e 80 00 20 blr
930: 39 20 00 00 li r9,0
934: 0b 09 00 00 tdnei r9,0
938: 4e 80 00 20 blr
We should keep things as they are and change the coccinelle script.
Christophe
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH linux-next] power:pkeys: fix bugon.cocci warnings
2022-05-07 7:04 ` Christophe Leroy
@ 2022-05-07 8:49 ` Arnd Bergmann
0 siblings, 0 replies; 5+ messages in thread
From: Arnd Bergmann @ 2022-05-07 8:49 UTC (permalink / raw)
To: Christophe Leroy
Cc: Michael Ellerman, CGEL, Benjamin Herrenschmidt, Paul Mackerras,
Sandipan Das, linuxppc-dev, linux-kernel, Zeal Robot,
Jing Yangyang
On Sat, May 7, 2022 at 9:04 AM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
> Le 02/05/2022 à 15:24, Michael Ellerman a écrit :
> > CGEL <cgel.zte@gmail.com> writes:
> >> From: Jing Yangyang <jing.yangyang@zte.com.cn>
> >>
> >> Use BUG_ON instead of a if condition followed by BUG.
> >>
> >> ./arch/powerpc/include/asm/book3s/64/pkeys.h:21:2-5:WARNING
> >> Use BUG_ON instead of if condition followed by BUG.
> >> ./arch/powerpc/include/asm/book3s/64/pkeys.h:14:2-5:WARNING
> >> Use BUG_ON instead of if condition followed by BUG.
> >>
> >> Generated by: scripts/coccinelle/misc/bugon.cocci
> >>
> >> Reported-by: Zeal Robot <zealci@zte.com.cn>
> >> Signed-off-by: Jing Yangyang <jing.yangyang@zte.com.cn>
> >> ---
> >> arch/powerpc/include/asm/book3s/64/pkeys.h | 6 ++----
> >> 1 file changed, 2 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/arch/powerpc/include/asm/book3s/64/pkeys.h b/arch/powerpc/include/asm/book3s/64/pkeys.h
> >> index 5b17813..5f74f0c 100644
> >> --- a/arch/powerpc/include/asm/book3s/64/pkeys.h
> >> +++ b/arch/powerpc/include/asm/book3s/64/pkeys.h
> >> @@ -10,15 +10,13 @@ static inline u64 vmflag_to_pte_pkey_bits(u64 vm_flags)
> >> if (!mmu_has_feature(MMU_FTR_PKEY))
> >> return 0x0UL;
> >>
> >> - if (radix_enabled())
> >> - BUG();
> >> + BUG_ON(radix_enabled());
> >> return hash__vmflag_to_pte_pkey_bits(vm_flags);
> >> }
> >>
> >> static inline u16 pte_to_pkey_bits(u64 pteflags)
> >> {
> >> - if (radix_enabled())
> >> - BUG();
> >> + BUG_ON(radix_enabled());
> >> return hash__pte_to_pkey_bits(pteflags);
> >> }
> >
> > Have you checked how this changes the generated code?
> >
> > radix_enabled() is a jump label, via mmu_feature().
> >
> > Possibly the compiler just works it all out and generates the same code,
> > but I'd want some evidence of that before merging this.
>
> Seems like the compiler is not that good, the generated code for test1()
> is much better than the one for test2(), see below.
>
> void test1(void)
> {
> if (radix_enabled())
> BUG();
> }
>
> void test2(void)
> {
> BUG_ON(radix_enabled());
> }
>
> 0000000000000900 <.test1>:
> 900: 60 00 00 00 nop
> 904: 0f e0 00 00 twui r0,0
> 908: 60 00 00 00 nop
> 90c: 60 00 00 00 nop
> 910: 4e 80 00 20 blr
> 914: 60 00 00 00 nop
> 918: 60 00 00 00 nop
> 91c: 60 00 00 00 nop
>
> 0000000000000920 <.test2>:
> 920: 60 00 00 00 nop
> 924: 39 20 00 01 li r9,1
> 928: 0b 09 00 00 tdnei r9,0
> 92c: 4e 80 00 20 blr
> 930: 39 20 00 00 li r9,0
> 934: 0b 09 00 00 tdnei r9,0
> 938: 4e 80 00 20 blr
>
>
> We should keep things as they are and change the coccinelle script.
Maybe just drop the custom ppc64 BUG_ON() then if it creates worse
code? The default BUG_ON() should be equivalent to the open-coded
version.
Arnd
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-05-07 8:49 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-25 6:42 [PATCH linux-next] power:pkeys: fix bugon.cocci warnings CGEL
2021-08-31 6:01 ` Daniel Axtens
2022-05-02 13:24 ` Michael Ellerman
2022-05-07 7:04 ` Christophe Leroy
2022-05-07 8:49 ` Arnd Bergmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).