[PATCH 3/4] mm: Fix demotion gfp by clearing GFP_RECLAIM after setting GFP_TRANSHUGE

[PATCH 3/4] mm: Fix demotion gfp by clearing GFP_RECLAIM after setting GFP_TRANSHUGE

[Alexandre Ghiti]

GFP_TRANSHUGE sets __GFP_DIRECT_RECLAIM so we must clear GFP_RECLAIM
after, not before.

Reported-by: Bing Jiao <bingjiao@google.com>
Closes: https://lore.kernel.org/linux-mm/aXlKOxGGI9zne8sl@google.com/
Fixes: 9933a0c8a539 ("mm/migrate: clear __GFP_RECLAIM to make the migration callback consistent with regular THP allocations")
Cc: stable@vger.kernel.org
Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
---
 mm/migrate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/migrate.c b/mm/migrate.c
index 2c3d489ecf51..ee533a4d38db 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -2190,12 +2190,12 @@ struct folio *alloc_migration_target(struct folio *src, unsigned long private)
 	}
 
 	if (folio_test_large(src)) {
+		gfp_mask |= GFP_TRANSHUGE;
 		/*
 		 * clear __GFP_RECLAIM to make the migration callback
 		 * consistent with regular THP allocations.
 		 */
 		gfp_mask &= ~__GFP_RECLAIM;
-		gfp_mask |= GFP_TRANSHUGE;
 		order = folio_order(src);
 	}
 	zidx = folio_zonenum(src);
-- 
2.53.0

[PATCH 4/4] mm: Fix demotion gfp by preserving initial gfp reclaim policy

[Alexandre Ghiti]

When the src folio is a hugetlb page, htlb_modify_alloc_mask() will
unconditionally enable reclaim. But we have to preserve initial gfp
flags which, in the case of demotion, prevent direct reclaim.

Reported-by: Gregory Price <gourry@gourry.net>
Closes: https://lore.kernel.org/linux-mm/aXkfBF5bdnTZ7t7e@gourry-fedora-PF4VCD3F/
Fixes: 19fc7bed252c ("mm/migrate: introduce a standard migration target allocation function")
Cc: stable@vger.kernel.org
Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
---
 mm/migrate.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/mm/migrate.c b/mm/migrate.c
index ee533a4d38db..d44a34d37007 100644
--- a/mm/migrate.c
+++ b/mm/migrate.c
@@ -2169,13 +2169,13 @@ int migrate_pages(struct list_head *from, new_folio_t get_new_folio,
 struct folio *alloc_migration_target(struct folio *src, unsigned long private)
 {
 	struct migration_target_control *mtc;
-	gfp_t gfp_mask;
+	gfp_t gfp_mask, gfp_entry;
 	unsigned int order = 0;
 	int nid;
 	enum zone_type zidx;
 
 	mtc = (struct migration_target_control *)private;
-	gfp_mask = mtc->gfp_mask;
+	gfp_mask = gfp_entry = mtc->gfp_mask;
 	nid = mtc->nid;
 	if (nid == NUMA_NO_NODE)
 		nid = folio_nid(src);
@@ -2184,6 +2184,8 @@ struct folio *alloc_migration_target(struct folio *src, unsigned long private)
 		struct hstate *h = folio_hstate(src);
 
 		gfp_mask = htlb_modify_alloc_mask(h, gfp_mask);
+		gfp_mask = (gfp_mask & ~__GFP_RECLAIM) | (gfp_entry & __GFP_RECLAIM);
+
 		return alloc_hugetlb_folio_nodemask(h, nid,
 						mtc->nmask, gfp_mask,
 						htlb_allow_alloc_fallback(mtc->reason));
-- 
2.53.0

Re: [PATCH 3/4] mm: Fix demotion gfp by clearing GFP_RECLAIM after setting GFP_TRANSHUGE

[Andrew Morton]

On Wed, 11 Mar 2026 12:02:42 +0100 Alexandre Ghiti <alex@ghiti.fr> wrote:

> Fixes: 9933a0c8a539 ("mm/migrate: clear __GFP_RECLAIM to make the migration callback consistent with regular THP allocations")
> Cc: stable@vger.kernel.org

Please let's have the cc:stable fixes separated out from the cleanups,
and prepared against current -linus mainline.

Also, when proposing backportable fixes please ensure that the
changelogs carefully describe the userspace-visible runtime effects of
the bug.

Thanks.

Re: [PATCH 3/4] mm: Fix demotion gfp by clearing GFP_RECLAIM after setting GFP_TRANSHUGE

[Johannes Weiner]

On Wed, Mar 11, 2026 at 12:02:42PM +0100, Alexandre Ghiti wrote:
> GFP_TRANSHUGE sets __GFP_DIRECT_RECLAIM so we must clear GFP_RECLAIM
> after, not before.
> 
> Reported-by: Bing Jiao <bingjiao@google.com>
> Closes: https://lore.kernel.org/linux-mm/aXlKOxGGI9zne8sl@google.com/
> Fixes: 9933a0c8a539 ("mm/migrate: clear __GFP_RECLAIM to make the migration callback consistent with regular THP allocations")
> Cc: stable@vger.kernel.org
> Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
> ---
>  mm/migrate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/migrate.c b/mm/migrate.c
> index 2c3d489ecf51..ee533a4d38db 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -2190,12 +2190,12 @@ struct folio *alloc_migration_target(struct folio *src, unsigned long private)
>  	}
>  
>  	if (folio_test_large(src)) {
> +		gfp_mask |= GFP_TRANSHUGE;
>  		/*
>  		 * clear __GFP_RECLAIM to make the migration callback
>  		 * consistent with regular THP allocations.
>  		 */
>  		gfp_mask &= ~__GFP_RECLAIM;
> -		gfp_mask |= GFP_TRANSHUGE;

I don't think this is right.

The Fixes: did it this way to disable kswapd for THP allocations,
while still allowing the customary direct reclaim. Maybe a better
comment would have been: /* GFP_TRANSHUGE has its own reclaim policy */

After your fix, direct reclaim isn't allowed either, which makes the
request unnecessarily wimpy.

The Closes: refers to reclaim that should be avoided during demotion.
But if this path is taken during demotion it will already not recurse
into direct reclaim due to PF_MEMALLOC.

So I don't see a bug in the existing code. But maybe the comment could
be clearer.

Re: [PATCH 3/4] mm: Fix demotion gfp by clearing GFP_RECLAIM after setting GFP_TRANSHUGE

[David Hildenbrand (Arm)]

On 3/11/26 18:06, Andrew Morton wrote:
> On Wed, 11 Mar 2026 12:02:42 +0100 Alexandre Ghiti <alex@ghiti.fr> wrote:
> 
>> Fixes: 9933a0c8a539 ("mm/migrate: clear __GFP_RECLAIM to make the migration callback consistent with regular THP allocations")
>> Cc: stable@vger.kernel.org
> 
> Please let's have the cc:stable fixes separated out from the cleanups,
> and prepared against current -linus mainline.
> 
> Also, when proposing backportable fixes please ensure that the
> changelogs carefully describe the userspace-visible runtime effects of
> the bug.

Also, please move fixes to the very beginning of the patch set, or
better, send them independently.

-- 
Cheers,

David

Re: [PATCH 3/4] mm: Fix demotion gfp by clearing GFP_RECLAIM after setting GFP_TRANSHUGE

[Gregory Price]

On Wed, Mar 11, 2026 at 01:54:50PM -0400, Johannes Weiner wrote:
> On Wed, Mar 11, 2026 at 12:02:42PM +0100, Alexandre Ghiti wrote:
> > GFP_TRANSHUGE sets __GFP_DIRECT_RECLAIM so we must clear GFP_RECLAIM
> > after, not before.
> > 
> > Reported-by: Bing Jiao <bingjiao@google.com>
> > Closes: https://lore.kernel.org/linux-mm/aXlKOxGGI9zne8sl@google.com/
> > Fixes: 9933a0c8a539 ("mm/migrate: clear __GFP_RECLAIM to make the migration callback consistent with regular THP allocations")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
> > ---
> >  mm/migrate.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/mm/migrate.c b/mm/migrate.c
> > index 2c3d489ecf51..ee533a4d38db 100644
> > --- a/mm/migrate.c
> > +++ b/mm/migrate.c
> > @@ -2190,12 +2190,12 @@ struct folio *alloc_migration_target(struct folio *src, unsigned long private)
> >  	}
> >  
> >  	if (folio_test_large(src)) {
> > +		gfp_mask |= GFP_TRANSHUGE;
> >  		/*
> >  		 * clear __GFP_RECLAIM to make the migration callback
> >  		 * consistent with regular THP allocations.
> >  		 */
> >  		gfp_mask &= ~__GFP_RECLAIM;
> > -		gfp_mask |= GFP_TRANSHUGE;
> 
> I don't think this is right.
> 
> The Fixes: did it this way to disable kswapd for THP allocations,
> while still allowing the customary direct reclaim. Maybe a better
> comment would have been: /* GFP_TRANSHUGE has its own reclaim policy */
> 

The bigger issue how many times we see this particular flag getting
masked and apparently added back in at multiple layers. We saw two
or three paths (some unreachable) that can twiddle RECLAIM flags in
the stack for demotion (which is in reclaim already, so do the flags
matter?).

It makes it difficult to reason about what the GFP flags actually
are at any given point.

But yeah I wasn't sure to make of this code, it could be as you
suggested just a bad comment.

~Gregory

Re: [PATCH 3/4] mm: Fix demotion gfp by clearing GFP_RECLAIM after setting GFP_TRANSHUGE

[Alexandre Ghiti]

Hi Andrew,

On 3/11/26 18:06, Andrew Morton wrote:
> On Wed, 11 Mar 2026 12:02:42 +0100 Alexandre Ghiti <alex@ghiti.fr> wrote:
>
>> Fixes: 9933a0c8a539 ("mm/migrate: clear __GFP_RECLAIM to make the migration callback consistent with regular THP allocations")
>> Cc: stable@vger.kernel.org
> Please let's have the cc:stable fixes separated out from the cleanups,
> and prepared against current -linus mainline.


I'll split the series in the next version.


>
> Also, when proposing backportable fixes please ensure that the
> changelogs carefully describe the userspace-visible runtime effects of
> the bug.


I was unaware of that requirement, I'll do.

Thanks,

Alex


>
> Thanks.

Re: [PATCH 3/4] mm: Fix demotion gfp by clearing GFP_RECLAIM after setting GFP_TRANSHUGE

[Alexandre Ghiti]

Hi Johannes,

On 3/11/26 18:54, Johannes Weiner wrote:
> On Wed, Mar 11, 2026 at 12:02:42PM +0100, Alexandre Ghiti wrote:
>> GFP_TRANSHUGE sets __GFP_DIRECT_RECLAIM so we must clear GFP_RECLAIM
>> after, not before.
>>
>> Reported-by: Bing Jiao <bingjiao@google.com>
>> Closes: https://lore.kernel.org/linux-mm/aXlKOxGGI9zne8sl@google.com/
>> Fixes: 9933a0c8a539 ("mm/migrate: clear __GFP_RECLAIM to make the migration callback consistent with regular THP allocations")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
>> ---
>>   mm/migrate.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/mm/migrate.c b/mm/migrate.c
>> index 2c3d489ecf51..ee533a4d38db 100644
>> --- a/mm/migrate.c
>> +++ b/mm/migrate.c
>> @@ -2190,12 +2190,12 @@ struct folio *alloc_migration_target(struct folio *src, unsigned long private)
>>   	}
>>   
>>   	if (folio_test_large(src)) {
>> +		gfp_mask |= GFP_TRANSHUGE;
>>   		/*
>>   		 * clear __GFP_RECLAIM to make the migration callback
>>   		 * consistent with regular THP allocations.
>>   		 */
>>   		gfp_mask &= ~__GFP_RECLAIM;
>> -		gfp_mask |= GFP_TRANSHUGE;
> I don't think this is right.
>
> The Fixes: did it this way to disable kswapd for THP allocations,
> while still allowing the customary direct reclaim. Maybe a better
> comment would have been: /* GFP_TRANSHUGE has its own reclaim policy */
>
> After your fix, direct reclaim isn't allowed either, which makes the
> request unnecessarily wimpy.
>
> The Closes: refers to reclaim that should be avoided during demotion.
> But if this path is taken during demotion it will already not recurse
> into direct reclaim due to PF_MEMALLOC.
>
> So I don't see a bug in the existing code. But maybe the comment could
> be clearer.


Makes sense, I had not understood the comment indeed. I'll drop this fix 
in the next version then.

Thanks,

Alex