mm/internal.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
As shown in the commit message of commit 242b872239f6a7deacbc
("include/linux/once_lite.h: fix judgment in WARN_ONCE with clang"),
the code "unlikely(a && b)" may generate poor assembly code if it is
actually "unlikely(a) && unlikely(b)" or "unlikely(a) && b".
WARN_ON_ONCE_GFP() may be used in the hot path code:
1. The argument cond shoud be unlikely.
2. When "1." is true, !(gfp & __GFP_NOWARN) is unlikely, otherwise, a
WARN may be triggered, which is a very unlikely case.
3. When "1. && 2." is true, just like the implementation of WARN_ONCE(),
!__warned can be unlikely.
Reorder __ret_warn_once judgement to first and split out the unlikely()
in WARN_ON_ONCE_GFP() to optimize performance.
Cc: Mike Rapoport <rppt@kernel.org>
Cc: David Laight <david.laight.linux@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Xie Yuanbin <qq570070308@gmail.com>
---
mm/internal.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/mm/internal.h b/mm/internal.h
index 6e1162e13289..49d2b7a270d3 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -93,7 +93,8 @@ struct pagetable_move_control {
static bool __section(".data..once") __warned; \
int __ret_warn_once = !!(cond); \
\
- if (unlikely(!(gfp & __GFP_NOWARN) && __ret_warn_once && !__warned)) { \
+ if (unlikely(__ret_warn_once) && \
+ unlikely(!(gfp & __GFP_NOWARN)) && unlikely(!__warned)) { \
__warned = true; \
WARN_ON(1); \
} \
--
2.53.0
On Mon, Mar 09, 2026 at 11:38:11PM +0800, Xie Yuanbin wrote:
> As shown in the commit message of commit 242b872239f6a7deacbc
> ("include/linux/once_lite.h: fix judgment in WARN_ONCE with clang"),
> the code "unlikely(a && b)" may generate poor assembly code if it is
> actually "unlikely(a) && unlikely(b)" or "unlikely(a) && b".
Why fix this in multiple places in the kernel instead of once in clang?
On Mon, 9 Mar 2026 15:40:13 +0000, Matthew Wilcox wrote:
>On Mon, Mar 09, 2026 at 11:38:11PM +0800, Xie Yuanbin wrote:
>> As shown in the commit message of commit 242b872239f6a7deacbc
>> ("include/linux/once_lite.h: fix judgment in WARN_ONCE with clang"),
>> the code "unlikely(a && b)" may generate poor assembly code if it is
>> actually "unlikely(a) && unlikely(b)" or "unlikely(a) && b".
>
> Why fix this in multiple places in the kernel instead of once in clang?
If a and b is both unlikely, then "unlikely(a) && unlikely(b)" will
generate better code than "unlikely(a && b)". This is also true for gcc.
As for the issue of clang judging twice, I have already submitted it to
clang:
Link: https://github.com/llvm/llvm-project/issues/167117
However, even if clang fixes it, this optimization will not be merged
back to the old version of clang.
On 3/9/26 16:59, Xie Yuanbin wrote:
> On Mon, 9 Mar 2026 15:40:13 +0000, Matthew Wilcox wrote:
>>On Mon, Mar 09, 2026 at 11:38:11PM +0800, Xie Yuanbin wrote:
>>> As shown in the commit message of commit 242b872239f6a7deacbc
>>> ("include/linux/once_lite.h: fix judgment in WARN_ONCE with clang"),
>>> the code "unlikely(a && b)" may generate poor assembly code if it is
>>> actually "unlikely(a) && unlikely(b)" or "unlikely(a) && b".
>>
>> Why fix this in multiple places in the kernel instead of once in clang?
>
> If a and b is both unlikely, then "unlikely(a) && unlikely(b)" will
> generate better code than "unlikely(a && b)". This is also true for gcc.
What are the details of how it's better for gcc?
> As for the issue of clang judging twice, I have already submitted it to
> clang:
> Link: https://github.com/llvm/llvm-project/issues/167117
> However, even if clang fixes it, this optimization will not be merged
> back to the old version of clang.
That's life and not worth complicating the kernel code for. This is not
about making it functional, only about perf.
On Tue, 10 Mar 2026 11:55:55 +0100
"Vlastimil Babka (SUSE)" <vbabka@kernel.org> wrote:
> On 3/9/26 16:59, Xie Yuanbin wrote:
> > On Mon, 9 Mar 2026 15:40:13 +0000, Matthew Wilcox wrote:
> >>On Mon, Mar 09, 2026 at 11:38:11PM +0800, Xie Yuanbin wrote:
> >>> As shown in the commit message of commit 242b872239f6a7deacbc
> >>> ("include/linux/once_lite.h: fix judgment in WARN_ONCE with clang"),
> >>> the code "unlikely(a && b)" may generate poor assembly code if it is
> >>> actually "unlikely(a) && unlikely(b)" or "unlikely(a) && b".
> >>
> >> Why fix this in multiple places in the kernel instead of once in clang?
> >
> > If a and b is both unlikely, then "unlikely(a) && unlikely(b)" will
> > generate better code than "unlikely(a && b)". This is also true for gcc.
>
> What are the details of how it's better for gcc?
I'm not sure about that specific case, but I've definitely seen gcc
generate sub-optimal code for some un/likely() of compound expressions.
The underlying cause is that the code is (probably) first transformed to:
bool tmp = expression;
if (unlikely(tmp)) ...
this means that you lose some of the short-circuiting that happens
early in the code generation of 'if (expression)'.
It is also not at all clear what you want the compiler to generate.
For 'unlikely(a || b)' you want 'if (a) goto x; if (b) goto x' so that
the 'likely' path is the no-branch one.
But for 'unlikely(a && b)' you still want 'if (a) goto x; y:' which means
that the 'b' test is out-of-line and has to be 'x: if (!b) goto y' to
avoid a branch when a is false - but that means you have a 'normally
taken' branch after the test of b.
That pretty much means the compiler has to decide which unlikely()
to ignore.
So it only makes sense to do 'if (unlikely(a) && b)'.
Indeed even 'if (unlikely(a) && likely(b))' may be better!
David
>
> > As for the issue of clang judging twice, I have already submitted it to
> > clang:
> > Link: https://github.com/llvm/llvm-project/issues/167117
> > However, even if clang fixes it, this optimization will not be merged
> > back to the old version of clang.
>
> That's life and not worth complicating the kernel code for. This is not
> about making it functional, only about perf.
>
On Tue, 10 Mar 2026 14:52:31 +0000 David Laight <david.laight.linux@gmail.com> wrote: > > > If a and b is both unlikely, then "unlikely(a) && unlikely(b)" will > > > generate better code than "unlikely(a && b)". This is also true for gcc. > > > > What are the details of how it's better for gcc? > > I'm not sure about that specific case, but I've definitely seen gcc > generate sub-optimal code for some un/likely() of compound expressions. > The underlying cause is that the code is (probably) first transformed to: > bool tmp = expression; > if (unlikely(tmp)) ... > this means that you lose some of the short-circuiting that happens > early in the code generation of 'if (expression)'. > > It is also not at all clear what you want the compiler to generate. > For 'unlikely(a || b)' you want 'if (a) goto x; if (b) goto x' so that > the 'likely' path is the no-branch one. > But for 'unlikely(a && b)' you still want 'if (a) goto x; y:' which means > that the 'b' test is out-of-line and has to be 'x: if (!b) goto y' to > avoid a branch when a is false - but that means you have a 'normally > taken' branch after the test of b. > That pretty much means the compiler has to decide which unlikely() > to ignore. > So it only makes sense to do 'if (unlikely(a) && b)'. > Indeed even 'if (unlikely(a) && likely(b))' may be better! fwiw, this change makes no change to `size mm/page_alloc.o' for x86_64 gcc defconfig. Given the expressed objections and that WARN_ON_ONCE_GFP() is used only twice in the whole kernel, I think I'll remove this patch. Xie, if you disagree with this then please resubmit the patch with a more convincing justification and hopefully people will reconsider it. Thanks.
© 2016 - 2026 Red Hat, Inc.