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@xxxxxxxxxx>
> > Closes: https://lore.kernel.org/linux-mm/aXlKOxGGI9zne8sl@xxxxxxxxxx/
> > Fixes: 9933a0c8a539 ("mm/migrate: clear __GFP_RECLAIM to make the migration callback consistent with regular THP allocations")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Alexandre Ghiti <alex@xxxxxxxx>
> > ---
> >   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