linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mm/page_alloc: only set ALLOC_HIGHATOMIC for __GPF_HIGH allocations
@ 2025-08-14 17:22 Thadeu Lima de Souza Cascardo
  2025-08-14 20:12 ` Johannes Weiner
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Thadeu Lima de Souza Cascardo @ 2025-08-14 17:22 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-mm, Andrew Morton, Vlastimil Babka, Suren Baghdasaryan,
	Michal Hocko, Brendan Jackman, Johannes Weiner, Zi Yan,
	Mel Gorman, kernel-dev, Thadeu Lima de Souza Cascardo,
	Helen Koike, Matthew Wilcox, NeilBrown, Thierry Reding

Commit 524c48072e56 ("mm/page_alloc: rename ALLOC_HIGH to
ALLOC_MIN_RESERVE") is the start of a series that explains how __GFP_HIGH,
which implies ALLOC_MIN_RESERVE, is going to be used instead of
__GFP_ATOMIC for high atomic reserves.

Commit eb2e2b425c69 ("mm/page_alloc: explicitly record high-order atomic
allocations in alloc_flags") introduced ALLOC_HIGHATOMIC for such
allocations of order higher than 0. It still used __GFP_ATOMIC, though.

Then, commit 1ebbb21811b7 ("mm/page_alloc: explicitly define how __GFP_HIGH
non-blocking allocations accesses reserves") just turned that check for
!__GFP_DIRECT_RECLAIM, ignoring that high atomic reserves were expected to
test for __GFP_HIGH.

This leads to high atomic reserves being added for high-order GFP_NOWAIT
allocations and others that clear __GFP_DIRECT_RECLAIM, which is
unexpected. Later, those reserves lead to 0-order allocations going to the
slow path and starting reclaim.

From /proc/pagetypeinfo, without the patch:

Node    0, zone      DMA, type   HighAtomic      0      0      0      0      0      0      0      0      0      0      0
Node    0, zone    DMA32, type   HighAtomic      1      8     10      9      7      3      0      0      0      0      0
Node    0, zone   Normal, type   HighAtomic     64     20     12      5      0      0      0      0      0      0      0

With the patch:

Node    0, zone      DMA, type   HighAtomic      0      0      0      0      0      0      0      0      0      0      0
Node    0, zone    DMA32, type   HighAtomic      0      0      0      0      0      0      0      0      0      0      0
Node    0, zone   Normal, type   HighAtomic      0      0      0      0      0      0      0      0      0      0      0

Fixes: 1ebbb21811b7 ("mm/page_alloc: explicitly define how __GFP_HIGH non-blocking allocations accesses reserves")
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
Tested-by: Helen Koike <koike@igalia.com>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Matthew Wilcox <willy@infradead.org>
Cc: NeilBrown <neilb@suse.de>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
---
 mm/page_alloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 2ef3c07266b3..bf52e3bef626 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4219,7 +4219,7 @@ gfp_to_alloc_flags(gfp_t gfp_mask, unsigned int order)
 		if (!(gfp_mask & __GFP_NOMEMALLOC)) {
 			alloc_flags |= ALLOC_NON_BLOCK;
 
-			if (order > 0)
+			if (order > 0 && (alloc_flags & ALLOC_MIN_RESERVE))
 				alloc_flags |= ALLOC_HIGHATOMIC;
 		}
 
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] mm/page_alloc: only set ALLOC_HIGHATOMIC for __GPF_HIGH allocations
  2025-08-14 17:22 [PATCH] mm/page_alloc: only set ALLOC_HIGHATOMIC for __GPF_HIGH allocations Thadeu Lima de Souza Cascardo
@ 2025-08-14 20:12 ` Johannes Weiner
  2025-08-14 20:34   ` Thadeu Lima de Souza Cascardo
  2025-08-28  5:06 ` Sergey Senozhatsky
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Johannes Weiner @ 2025-08-14 20:12 UTC (permalink / raw)
  To: Thadeu Lima de Souza Cascardo
  Cc: linux-kernel, linux-mm, Andrew Morton, Vlastimil Babka,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman, Zi Yan,
	Mel Gorman, kernel-dev, Helen Koike, Matthew Wilcox, NeilBrown,
	Thierry Reding

Hello Thadeu,

On Thu, Aug 14, 2025 at 02:22:45PM -0300, Thadeu Lima de Souza Cascardo wrote:
> Commit 524c48072e56 ("mm/page_alloc: rename ALLOC_HIGH to
> ALLOC_MIN_RESERVE") is the start of a series that explains how __GFP_HIGH,
> which implies ALLOC_MIN_RESERVE, is going to be used instead of
> __GFP_ATOMIC for high atomic reserves.
> 
> Commit eb2e2b425c69 ("mm/page_alloc: explicitly record high-order atomic
> allocations in alloc_flags") introduced ALLOC_HIGHATOMIC for such
> allocations of order higher than 0. It still used __GFP_ATOMIC, though.
> 
> Then, commit 1ebbb21811b7 ("mm/page_alloc: explicitly define how __GFP_HIGH
> non-blocking allocations accesses reserves") just turned that check for
> !__GFP_DIRECT_RECLAIM, ignoring that high atomic reserves were expected to
> test for __GFP_HIGH.

It indeed looks accidental. From the cover letter,

    High-order atomic allocations are explicitly handled with the caveat that
    no __GFP_ATOMIC flag means that any high-order allocation that specifies
    GFP_HIGH and cannot enter direct reclaim will be treated as if it was
    GFP_ATOMIC.

it sounds like the intent was what your patch does, and not to extend
those privileges to anybody who is !gfp_direct_reclaim.

> This leads to high atomic reserves being added for high-order GFP_NOWAIT
> allocations and others that clear __GFP_DIRECT_RECLAIM, which is
> unexpected. Later, those reserves lead to 0-order allocations going to the
> slow path and starting reclaim.

Can you please provide more background on the workload and the
environment in which you observed this?

Which GFP_NOWAIT requests you saw participating in the reserves etc.

I would feel better with Mel or Vlastimil chiming in as well, but your
fix looks correct to me.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] mm/page_alloc: only set ALLOC_HIGHATOMIC for __GPF_HIGH allocations
  2025-08-14 20:12 ` Johannes Weiner
@ 2025-08-14 20:34   ` Thadeu Lima de Souza Cascardo
  0 siblings, 0 replies; 10+ messages in thread
From: Thadeu Lima de Souza Cascardo @ 2025-08-14 20:34 UTC (permalink / raw)
  To: Johannes Weiner
  Cc: linux-kernel, linux-mm, Andrew Morton, Vlastimil Babka,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman, Zi Yan,
	Mel Gorman, kernel-dev, Helen Koike, Matthew Wilcox, NeilBrown,
	Thierry Reding

On Thu, Aug 14, 2025 at 04:12:11PM -0400, Johannes Weiner wrote:
> Hello Thadeu,
> 
> On Thu, Aug 14, 2025 at 02:22:45PM -0300, Thadeu Lima de Souza Cascardo wrote:
> > Commit 524c48072e56 ("mm/page_alloc: rename ALLOC_HIGH to
> > ALLOC_MIN_RESERVE") is the start of a series that explains how __GFP_HIGH,
> > which implies ALLOC_MIN_RESERVE, is going to be used instead of
> > __GFP_ATOMIC for high atomic reserves.
> > 
> > Commit eb2e2b425c69 ("mm/page_alloc: explicitly record high-order atomic
> > allocations in alloc_flags") introduced ALLOC_HIGHATOMIC for such
> > allocations of order higher than 0. It still used __GFP_ATOMIC, though.
> > 
> > Then, commit 1ebbb21811b7 ("mm/page_alloc: explicitly define how __GFP_HIGH
> > non-blocking allocations accesses reserves") just turned that check for
> > !__GFP_DIRECT_RECLAIM, ignoring that high atomic reserves were expected to
> > test for __GFP_HIGH.
> 
> It indeed looks accidental. From the cover letter,
> 
>     High-order atomic allocations are explicitly handled with the caveat that
>     no __GFP_ATOMIC flag means that any high-order allocation that specifies
>     GFP_HIGH and cannot enter direct reclaim will be treated as if it was
>     GFP_ATOMIC.
> 
> it sounds like the intent was what your patch does, and not to extend
> those privileges to anybody who is !gfp_direct_reclaim.
> 
> > This leads to high atomic reserves being added for high-order GFP_NOWAIT
> > allocations and others that clear __GFP_DIRECT_RECLAIM, which is
> > unexpected. Later, those reserves lead to 0-order allocations going to the
> > slow path and starting reclaim.
> 
> Can you please provide more background on the workload and the
> environment in which you observed this?
> 
> Which GFP_NOWAIT requests you saw participating in the reserves etc.
> 
> I would feel better with Mel or Vlastimil chiming in as well, but your
> fix looks correct to me.

Thanks for the review, Johannes.

This has been observed in a browser/desktop environment test, where we have
noticed some memory pressure regression. This change alone does not make
the regression go away entirely, but it improves it.

I noticed some unix skb allocation going on and I found this at
net/core/skbuff.c:alloc_skb_with_frags:

			page = alloc_pages((gfp_mask & ~__GFP_DIRECT_RECLAIM) |
					   __GFP_COMP |
					   __GFP_NOWARN,
					   order);

But I tested this at a simple VM with the most simple workload (no swap,
writing to tmpfs) and it triggered with xarrays. At lib/xarray.c:xas_alloc:

		gfp_t gfp = GFP_NOWAIT | __GFP_NOWARN;

		if (xas->xa->xa_flags & XA_FLAGS_ACCOUNT)
			gfp |= __GFP_ACCOUNT;

		node = kmem_cache_alloc_lru(radix_tree_node_cachep, xas->xa_lru, gfp);

Where radix_tree_node_cachep, on that VM, uses a 4-page slab.

I tested with something like:

			if (order > 0) {
				WARN_ON_ONCE(!(alloc_flags & ALLOC_MIN_RESERVE));
				alloc_flags |= ALLOC_HIGHATOMIC;
			}

Thanks.
Cascardo.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] mm/page_alloc: only set ALLOC_HIGHATOMIC for __GPF_HIGH allocations
  2025-08-14 17:22 [PATCH] mm/page_alloc: only set ALLOC_HIGHATOMIC for __GPF_HIGH allocations Thadeu Lima de Souza Cascardo
  2025-08-14 20:12 ` Johannes Weiner
@ 2025-08-28  5:06 ` Sergey Senozhatsky
  2025-08-29  6:41 ` Michal Hocko
  2025-08-29  8:36 ` Vlastimil Babka
  3 siblings, 0 replies; 10+ messages in thread
From: Sergey Senozhatsky @ 2025-08-28  5:06 UTC (permalink / raw)
  To: Vlastimil Babka, Mel Gorman, Andrew Morton
  Cc: linux-kernel, linux-mm, Suren Baghdasaryan, Michal Hocko,
	Brendan Jackman, Johannes Weiner, Zi Yan, kernel-dev, Helen Koike,
	Matthew Wilcox, NeilBrown, Thierry Reding,
	Thadeu Lima de Souza Cascardo

On (25/08/14 14:22), Thadeu Lima de Souza Cascardo wrote:
> Commit 524c48072e56 ("mm/page_alloc: rename ALLOC_HIGH to
> ALLOC_MIN_RESERVE") is the start of a series that explains how __GFP_HIGH,
> which implies ALLOC_MIN_RESERVE, is going to be used instead of
> __GFP_ATOMIC for high atomic reserves.
> 
> Commit eb2e2b425c69 ("mm/page_alloc: explicitly record high-order atomic
> allocations in alloc_flags") introduced ALLOC_HIGHATOMIC for such
> allocations of order higher than 0. It still used __GFP_ATOMIC, though.
> 
> Then, commit 1ebbb21811b7 ("mm/page_alloc: explicitly define how __GFP_HIGH
> non-blocking allocations accesses reserves") just turned that check for
> !__GFP_DIRECT_RECLAIM, ignoring that high atomic reserves were expected to
> test for __GFP_HIGH.
> 
> This leads to high atomic reserves being added for high-order GFP_NOWAIT
> allocations and others that clear __GFP_DIRECT_RECLAIM, which is
> unexpected. Later, those reserves lead to 0-order allocations going to the
> slow path and starting reclaim.
> 
> From /proc/pagetypeinfo, without the patch:
> 
> Node    0, zone      DMA, type   HighAtomic      0      0      0      0      0      0      0      0      0      0      0
> Node    0, zone    DMA32, type   HighAtomic      1      8     10      9      7      3      0      0      0      0      0
> Node    0, zone   Normal, type   HighAtomic     64     20     12      5      0      0      0      0      0      0      0
> 
> With the patch:
> 
> Node    0, zone      DMA, type   HighAtomic      0      0      0      0      0      0      0      0      0      0      0
> Node    0, zone    DMA32, type   HighAtomic      0      0      0      0      0      0      0      0      0      0      0
> Node    0, zone   Normal, type   HighAtomic      0      0      0      0      0      0      0      0      0      0      0

[..]

> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 2ef3c07266b3..bf52e3bef626 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -4219,7 +4219,7 @@ gfp_to_alloc_flags(gfp_t gfp_mask, unsigned int order)
>  		if (!(gfp_mask & __GFP_NOMEMALLOC)) {
>  			alloc_flags |= ALLOC_NON_BLOCK;
>  
> -			if (order > 0)
> +			if (order > 0 && (alloc_flags & ALLOC_MIN_RESERVE))
>  				alloc_flags |= ALLOC_HIGHATOMIC;
>  		}

From my limited understanding, it does look like this was the
intention.  Vlastimil, Mel, got a minute to take a look?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] mm/page_alloc: only set ALLOC_HIGHATOMIC for __GPF_HIGH allocations
  2025-08-14 17:22 [PATCH] mm/page_alloc: only set ALLOC_HIGHATOMIC for __GPF_HIGH allocations Thadeu Lima de Souza Cascardo
  2025-08-14 20:12 ` Johannes Weiner
  2025-08-28  5:06 ` Sergey Senozhatsky
@ 2025-08-29  6:41 ` Michal Hocko
  2025-08-29  8:36 ` Vlastimil Babka
  3 siblings, 0 replies; 10+ messages in thread
From: Michal Hocko @ 2025-08-29  6:41 UTC (permalink / raw)
  To: Thadeu Lima de Souza Cascardo
  Cc: linux-kernel, linux-mm, Andrew Morton, Vlastimil Babka,
	Suren Baghdasaryan, Brendan Jackman, Johannes Weiner, Zi Yan,
	Mel Gorman, kernel-dev, Helen Koike, Matthew Wilcox, NeilBrown,
	Thierry Reding

On Thu 14-08-25 14:22:45, Thadeu Lima de Souza Cascardo wrote:
> Commit 524c48072e56 ("mm/page_alloc: rename ALLOC_HIGH to
> ALLOC_MIN_RESERVE") is the start of a series that explains how __GFP_HIGH,
> which implies ALLOC_MIN_RESERVE, is going to be used instead of
> __GFP_ATOMIC for high atomic reserves.
> 
> Commit eb2e2b425c69 ("mm/page_alloc: explicitly record high-order atomic
> allocations in alloc_flags") introduced ALLOC_HIGHATOMIC for such
> allocations of order higher than 0. It still used __GFP_ATOMIC, though.
> 
> Then, commit 1ebbb21811b7 ("mm/page_alloc: explicitly define how __GFP_HIGH
> non-blocking allocations accesses reserves") just turned that check for
> !__GFP_DIRECT_RECLAIM, ignoring that high atomic reserves were expected to
> test for __GFP_HIGH.
> 
> This leads to high atomic reserves being added for high-order GFP_NOWAIT
> allocations and others that clear __GFP_DIRECT_RECLAIM, which is
> unexpected. Later, those reserves lead to 0-order allocations going to the
> slow path and starting reclaim.
> 
> >From /proc/pagetypeinfo, without the patch:
> 
> Node    0, zone      DMA, type   HighAtomic      0      0      0      0      0      0      0      0      0      0      0
> Node    0, zone    DMA32, type   HighAtomic      1      8     10      9      7      3      0      0      0      0      0
> Node    0, zone   Normal, type   HighAtomic     64     20     12      5      0      0      0      0      0      0      0
> 
> With the patch:
> 
> Node    0, zone      DMA, type   HighAtomic      0      0      0      0      0      0      0      0      0      0      0
> Node    0, zone    DMA32, type   HighAtomic      0      0      0      0      0      0      0      0      0      0      0
> Node    0, zone   Normal, type   HighAtomic      0      0      0      0      0      0      0      0      0      0      0
> 
> Fixes: 1ebbb21811b7 ("mm/page_alloc: explicitly define how __GFP_HIGH non-blocking allocations accesses reserves")
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
> Tested-by: Helen Koike <koike@igalia.com>
> Cc: Mel Gorman <mgorman@techsingularity.net>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: NeilBrown <neilb@suse.de>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>

Yes, this makes a lot of sense to me. GFP_NOWAIT allocations should be
opportunistic and quick to fail rather than dipping into memory
reserves. We must have overlooked that during the review.
Acked-by: Michal Hocko <mhocko@suse.com>

> ---
>  mm/page_alloc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 2ef3c07266b3..bf52e3bef626 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -4219,7 +4219,7 @@ gfp_to_alloc_flags(gfp_t gfp_mask, unsigned int order)
>  		if (!(gfp_mask & __GFP_NOMEMALLOC)) {
>  			alloc_flags |= ALLOC_NON_BLOCK;
>  
> -			if (order > 0)
> +			if (order > 0 && (alloc_flags & ALLOC_MIN_RESERVE))
>  				alloc_flags |= ALLOC_HIGHATOMIC;
>  		}
>  
> -- 
> 2.47.2

-- 
Michal Hocko
SUSE Labs

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] mm/page_alloc: only set ALLOC_HIGHATOMIC for __GPF_HIGH allocations
  2025-08-14 17:22 [PATCH] mm/page_alloc: only set ALLOC_HIGHATOMIC for __GPF_HIGH allocations Thadeu Lima de Souza Cascardo
                   ` (2 preceding siblings ...)
  2025-08-29  6:41 ` Michal Hocko
@ 2025-08-29  8:36 ` Vlastimil Babka
  2025-08-29  8:38   ` Vlastimil Babka
  3 siblings, 1 reply; 10+ messages in thread
From: Vlastimil Babka @ 2025-08-29  8:36 UTC (permalink / raw)
  To: Thadeu Lima de Souza Cascardo, linux-kernel
  Cc: linux-mm, Andrew Morton, Suren Baghdasaryan, Michal Hocko,
	Brendan Jackman, Johannes Weiner, Zi Yan, Mel Gorman, kernel-dev,
	Helen Koike, Matthew Wilcox, Thierry Reding, Sergey Senozhatsky,
	neil

On 8/14/25 19:22, Thadeu Lima de Souza Cascardo wrote:
> Commit 524c48072e56 ("mm/page_alloc: rename ALLOC_HIGH to
> ALLOC_MIN_RESERVE") is the start of a series that explains how __GFP_HIGH,
> which implies ALLOC_MIN_RESERVE, is going to be used instead of
> __GFP_ATOMIC for high atomic reserves.
> 
> Commit eb2e2b425c69 ("mm/page_alloc: explicitly record high-order atomic
> allocations in alloc_flags") introduced ALLOC_HIGHATOMIC for such
> allocations of order higher than 0. It still used __GFP_ATOMIC, though.
> 
> Then, commit 1ebbb21811b7 ("mm/page_alloc: explicitly define how __GFP_HIGH
> non-blocking allocations accesses reserves") just turned that check for
> !__GFP_DIRECT_RECLAIM, ignoring that high atomic reserves were expected to
> test for __GFP_HIGH.
> 
> This leads to high atomic reserves being added for high-order GFP_NOWAIT
> allocations and others that clear __GFP_DIRECT_RECLAIM, which is
> unexpected. Later, those reserves lead to 0-order allocations going to the
> slow path and starting reclaim.
> 
> From /proc/pagetypeinfo, without the patch:
> 
> Node    0, zone      DMA, type   HighAtomic      0      0      0      0      0      0      0      0      0      0      0
> Node    0, zone    DMA32, type   HighAtomic      1      8     10      9      7      3      0      0      0      0      0
> Node    0, zone   Normal, type   HighAtomic     64     20     12      5      0      0      0      0      0      0      0
> 
> With the patch:
> 
> Node    0, zone      DMA, type   HighAtomic      0      0      0      0      0      0      0      0      0      0      0
> Node    0, zone    DMA32, type   HighAtomic      0      0      0      0      0      0      0      0      0      0      0
> Node    0, zone   Normal, type   HighAtomic      0      0      0      0      0      0      0      0      0      0      0
> 
> Fixes: 1ebbb21811b7 ("mm/page_alloc: explicitly define how __GFP_HIGH non-blocking allocations accesses reserves")
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
> Tested-by: Helen Koike <koike@igalia.com>
> Cc: Mel Gorman <mgorman@techsingularity.net>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: NeilBrown <neilb@suse.de>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Vlastimil Babka <vbabka@suse.cz>

Agreed with others that this change matches the original intention and it
must have been an oversight. Also found nothing to the contrary in the
original threads.

> ---
>  mm/page_alloc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 2ef3c07266b3..bf52e3bef626 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -4219,7 +4219,7 @@ gfp_to_alloc_flags(gfp_t gfp_mask, unsigned int order)
>  		if (!(gfp_mask & __GFP_NOMEMALLOC)) {
>  			alloc_flags |= ALLOC_NON_BLOCK;
>  
> -			if (order > 0)
> +			if (order > 0 && (alloc_flags & ALLOC_MIN_RESERVE))
>  				alloc_flags |= ALLOC_HIGHATOMIC;
>  		}
>  


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] mm/page_alloc: only set ALLOC_HIGHATOMIC for __GPF_HIGH allocations
  2025-08-29  8:36 ` Vlastimil Babka
@ 2025-08-29  8:38   ` Vlastimil Babka
  2025-08-29  8:56     ` Sergey Senozhatsky
  0 siblings, 1 reply; 10+ messages in thread
From: Vlastimil Babka @ 2025-08-29  8:38 UTC (permalink / raw)
  To: Thadeu Lima de Souza Cascardo, linux-kernel
  Cc: linux-mm, Andrew Morton, Suren Baghdasaryan, Michal Hocko,
	Brendan Jackman, Johannes Weiner, Zi Yan, Mel Gorman, kernel-dev,
	Helen Koike, Matthew Wilcox, Thierry Reding, Sergey Senozhatsky,
	neil

On 8/29/25 10:36, Vlastimil Babka wrote:
> On 8/14/25 19:22, Thadeu Lima de Souza Cascardo wrote:
>> Commit 524c48072e56 ("mm/page_alloc: rename ALLOC_HIGH to
>> ALLOC_MIN_RESERVE") is the start of a series that explains how __GFP_HIGH,
>> which implies ALLOC_MIN_RESERVE, is going to be used instead of
>> __GFP_ATOMIC for high atomic reserves.
>> 
>> Commit eb2e2b425c69 ("mm/page_alloc: explicitly record high-order atomic
>> allocations in alloc_flags") introduced ALLOC_HIGHATOMIC for such
>> allocations of order higher than 0. It still used __GFP_ATOMIC, though.
>> 
>> Then, commit 1ebbb21811b7 ("mm/page_alloc: explicitly define how __GFP_HIGH
>> non-blocking allocations accesses reserves") just turned that check for
>> !__GFP_DIRECT_RECLAIM, ignoring that high atomic reserves were expected to
>> test for __GFP_HIGH.
>> 
>> This leads to high atomic reserves being added for high-order GFP_NOWAIT
>> allocations and others that clear __GFP_DIRECT_RECLAIM, which is
>> unexpected. Later, those reserves lead to 0-order allocations going to the
>> slow path and starting reclaim.
>> 
>> From /proc/pagetypeinfo, without the patch:
>> 
>> Node    0, zone      DMA, type   HighAtomic      0      0      0      0      0      0      0      0      0      0      0
>> Node    0, zone    DMA32, type   HighAtomic      1      8     10      9      7      3      0      0      0      0      0
>> Node    0, zone   Normal, type   HighAtomic     64     20     12      5      0      0      0      0      0      0      0
>> 
>> With the patch:
>> 
>> Node    0, zone      DMA, type   HighAtomic      0      0      0      0      0      0      0      0      0      0      0
>> Node    0, zone    DMA32, type   HighAtomic      0      0      0      0      0      0      0      0      0      0      0
>> Node    0, zone   Normal, type   HighAtomic      0      0      0      0      0      0      0      0      0      0      0
>> 
>> Fixes: 1ebbb21811b7 ("mm/page_alloc: explicitly define how __GFP_HIGH non-blocking allocations accesses reserves")
>> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@igalia.com>
>> Tested-by: Helen Koike <koike@igalia.com>
>> Cc: Mel Gorman <mgorman@techsingularity.net>
>> Cc: Michal Hocko <mhocko@suse.com>
>> Cc: Matthew Wilcox <willy@infradead.org>
>> Cc: NeilBrown <neilb@suse.de>
>> Cc: Thierry Reding <thierry.reding@gmail.com>
>> Cc: Vlastimil Babka <vbabka@suse.cz>
> 
> Agreed with others that this change matches the original intention and it
> must have been an oversight. Also found nothing to the contrary in the
> original threads.

Oops, forgot to add

Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

>> ---
>>  mm/page_alloc.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 2ef3c07266b3..bf52e3bef626 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -4219,7 +4219,7 @@ gfp_to_alloc_flags(gfp_t gfp_mask, unsigned int order)
>>  		if (!(gfp_mask & __GFP_NOMEMALLOC)) {
>>  			alloc_flags |= ALLOC_NON_BLOCK;
>>  
>> -			if (order > 0)
>> +			if (order > 0 && (alloc_flags & ALLOC_MIN_RESERVE))
>>  				alloc_flags |= ALLOC_HIGHATOMIC;
>>  		}
>>  
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] mm/page_alloc: only set ALLOC_HIGHATOMIC for __GPF_HIGH allocations
  2025-08-29  8:38   ` Vlastimil Babka
@ 2025-08-29  8:56     ` Sergey Senozhatsky
  2025-08-29  9:30       ` Vlastimil Babka
  0 siblings, 1 reply; 10+ messages in thread
From: Sergey Senozhatsky @ 2025-08-29  8:56 UTC (permalink / raw)
  To: Vlastimil Babka, Andrew Morton, Thadeu Lima de Souza Cascardo
  Cc: linux-kernel, linux-mm, Suren Baghdasaryan, Michal Hocko,
	Brendan Jackman, Johannes Weiner, Zi Yan, Mel Gorman, kernel-dev,
	Helen Koike, Matthew Wilcox, Thierry Reding, Sergey Senozhatsky,
	neil

On (25/08/29 10:38), Vlastimil Babka wrote:
> On 8/29/25 10:36, Vlastimil Babka wrote:
> > On 8/14/25 19:22, Thadeu Lima de Souza Cascardo wrote:
[..]
> > Agreed with others that this change matches the original intention and it
> > must have been an oversight. Also found nothing to the contrary in the
> > original threads.
> 
> Oops, forgot to add
> 
> Reviewed-by: Vlastimil Babka <vbabka@suse.cz>

Thank you!

FWIW
Tested-by: Sergey Senozhatsky <senozhatsky@chromium.org>

This needs Cc: stable@vger.kernel.org  all the way down to 5.15,
as far as I can tell.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] mm/page_alloc: only set ALLOC_HIGHATOMIC for __GPF_HIGH allocations
  2025-08-29  8:56     ` Sergey Senozhatsky
@ 2025-08-29  9:30       ` Vlastimil Babka
  2025-08-29 10:30         ` Thadeu Lima de Souza Cascardo
  0 siblings, 1 reply; 10+ messages in thread
From: Vlastimil Babka @ 2025-08-29  9:30 UTC (permalink / raw)
  To: Sergey Senozhatsky, Andrew Morton, Thadeu Lima de Souza Cascardo
  Cc: linux-kernel, linux-mm, Suren Baghdasaryan, Michal Hocko,
	Brendan Jackman, Johannes Weiner, Zi Yan, Mel Gorman, kernel-dev,
	Helen Koike, Matthew Wilcox, Thierry Reding, neil

On 8/29/25 10:56, Sergey Senozhatsky wrote:
> On (25/08/29 10:38), Vlastimil Babka wrote:
>> On 8/29/25 10:36, Vlastimil Babka wrote:
>> > On 8/14/25 19:22, Thadeu Lima de Souza Cascardo wrote:
> [..]
>> > Agreed with others that this change matches the original intention and it
>> > must have been an oversight. Also found nothing to the contrary in the
>> > original threads.
>> 
>> Oops, forgot to add
>> 
>> Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
> 
> Thank you!
> 
> FWIW
> Tested-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> 
> This needs Cc: stable@vger.kernel.org  all the way down to 5.15,
> as far as I can tell.

If that problem bothers users of LTS kernels in that range, we can do that.
I wonder a bit how it was only found out now as a regression in the
browser/desktop environment test if it's that old? Is there another factor
i.e. some new frequesnt source of allocations that trigger it?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] mm/page_alloc: only set ALLOC_HIGHATOMIC for __GPF_HIGH allocations
  2025-08-29  9:30       ` Vlastimil Babka
@ 2025-08-29 10:30         ` Thadeu Lima de Souza Cascardo
  0 siblings, 0 replies; 10+ messages in thread
From: Thadeu Lima de Souza Cascardo @ 2025-08-29 10:30 UTC (permalink / raw)
  To: Vlastimil Babka
  Cc: Sergey Senozhatsky, Andrew Morton, linux-kernel, linux-mm,
	Suren Baghdasaryan, Michal Hocko, Brendan Jackman,
	Johannes Weiner, Zi Yan, Mel Gorman, kernel-dev, Helen Koike,
	Matthew Wilcox, Thierry Reding, neil

On Fri, Aug 29, 2025 at 11:30:17AM +0200, Vlastimil Babka wrote:
> On 8/29/25 10:56, Sergey Senozhatsky wrote:
> > On (25/08/29 10:38), Vlastimil Babka wrote:
> >> On 8/29/25 10:36, Vlastimil Babka wrote:
> >> > On 8/14/25 19:22, Thadeu Lima de Souza Cascardo wrote:
> > [..]
> >> > Agreed with others that this change matches the original intention and it
> >> > must have been an oversight. Also found nothing to the contrary in the
> >> > original threads.
> >> 
> >> Oops, forgot to add
> >> 
> >> Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
> > 
> > Thank you!
> > 
> > FWIW
> > Tested-by: Sergey Senozhatsky <senozhatsky@chromium.org>
> > 
> > This needs Cc: stable@vger.kernel.org  all the way down to 5.15,
> > as far as I can tell.
> 
> If that problem bothers users of LTS kernels in that range, we can do that.
> I wonder a bit how it was only found out now as a regression in the
> browser/desktop environment test if it's that old? Is there another factor
> i.e. some new frequesnt source of allocations that trigger it?

That is not. It is just that there was an upgrade all the way back from 5.4
and, then, I caught this while doing some code inspection and reviewing the
patchset I referred to.

Well, I also tested that it really happens and caught a unix socket skb
allocation triggering that as it masks off __GFP_DIRECT_RECLAIM.

Cascardo.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-08-29 10:30 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-14 17:22 [PATCH] mm/page_alloc: only set ALLOC_HIGHATOMIC for __GPF_HIGH allocations Thadeu Lima de Souza Cascardo
2025-08-14 20:12 ` Johannes Weiner
2025-08-14 20:34   ` Thadeu Lima de Souza Cascardo
2025-08-28  5:06 ` Sergey Senozhatsky
2025-08-29  6:41 ` Michal Hocko
2025-08-29  8:36 ` Vlastimil Babka
2025-08-29  8:38   ` Vlastimil Babka
2025-08-29  8:56     ` Sergey Senozhatsky
2025-08-29  9:30       ` Vlastimil Babka
2025-08-29 10:30         ` Thadeu Lima de Souza Cascardo

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