[PATCH] mm: optimize the implementation of WARN_ON_ONCE_GFP()

[PATCH] mm: optimize the implementation of WARN_ON_ONCE_GFP()

[Xie Yuanbin]

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

Re: [PATCH] mm: optimize the implementation of WARN_ON_ONCE_GFP()

[Matthew Wilcox]

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?

Re: [PATCH] mm: optimize the implementation of WARN_ON_ONCE_GFP()

[Xie Yuanbin]

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.

Re: [PATCH] mm: optimize the implementation of WARN_ON_ONCE_GFP()

[Vlastimil Babka (SUSE)]

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.

Re: [PATCH] mm: optimize the implementation of WARN_ON_ONCE_GFP()

[David Laight]

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.
> 

Re: [PATCH] mm: optimize the implementation of WARN_ON_ONCE_GFP()

[Andrew Morton]

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.