Re: [PATCH RESEND v2 4/6] mm/page_alloc: sort out the alloc_contig_range() gfp flags mess

[Vlastimil Babka <vbabka@suse.cz>]

On 12/3/24 20:19, David Hildenbrand wrote:
> On 03.12.24 15:24, Vlastimil Babka wrote:
>> On 12/3/24 15:12, David Hildenbrand wrote:
>>> On 03.12.24 14:55, Vlastimil Babka wrote:
>>> likely the thing we are assuming here is that we are migrating a page, and
>>> usually, these are user allocation (except maybe balloon and some other non-lru
>>> movable things).
>> 
>> Yeah and user allocations obey cpuset and mempolicies etc. But these are
>> likely somebody elses allocations that were done according to their
>> policies. With our migration we might be actually violating those, which
>> probably can't be helped (is at least migration within the same node
>> preferred? hmm).
> 
> I would hope that we handle memory policies somehow (via VMAs? not 
> sure). cpuset? I have no idea.

They are handled when allocating, but then the info is lost, the allocation
doesn't carry its effective nodemask.
But that's really a separate issue that just occured to me.

> But it doesn't seem to me that our caller's restrictions
>> (if those exist, would be enforced by __GFP_HARDWALL) are that relevant for
>> somebody else's pages?
> 
> It was always set using "GFP_USER | __GFP_MOVABLE | 
> __GFP_RETRY_MAYFAIL", and I removed the same flag combination in #2 from 
> memory offline code, and we do have the exact same thing in 
> do_migrate_range() in mm/memory_hotplug.c.

Yeah I agree a refactoring patch shouldn't change the existing behavior...

> We should investigate if__GFP_HARDWALL is the right thing to use here, 
> and if we can get rid of that by switching to GFP_KERNEL in all these 
> places.
> 
> I can look into it + send a follow-up patch.

...but it's a great opportunity to start questioning it and possibly change
it as a follow up :)

Thanks!

> Thanks!
>