Re: [PATCH] mm: Re-allow pinning of zero pfns

[Alistair Popple]

Looks like the original patch might need rebasing. I am about to post a
clean-up for the tortured logic in check_and_migrate_movable_pages() so
can incorporate it there, but I'm wondering what the consensus was for
pinning of zero pfn?

Currently my clean-up will result in PUP returning an error for the zero
pfn rather than looping indefinitely in the kernel. However it wasn't
clear from this thread if returning an error is ok, or if R/O pinning
of the zero pfn should succeed?

 - Alistair

Alistair Popple <apopple@xxxxxxxxxx> writes:

> Jason Gunthorpe <jgg@xxxxxxxxxx> writes:
>
>> On Fri, Jun 24, 2022 at 10:11:01AM +1000, Alistair Popple wrote:
>>
>>> > Hum.. Alistair, maybe you should look at this as well, I'm struggling
>>> > alot to understand how it is safe to drop the reference on the page
>>> > but hold a pointer to it on the movable_page_list - sure it was
>>> > isolated - but why does that mean it won't be concurrently unmapped
>>> > and freed?
>>>
>>> folio_isolate_lru() takes a reference on the page so you're safe from it
>>> being freed. If it gets unmapped it will be freed when the matching
>>> putback_movable_pages() is called.
>>
>> Hm, I guess I didn't dig deep enough into that call chain..
>>
>>> > Anyhow, it looks like the problem is the tortured logic in this
>>> > function, what do you think about this:
>>>
>>> At a glance it seems reasonable, although I fear it might conflict with
>>> my changes for device coherent migration. Agree the whole
>>> check_and_migrate_movable_pages() logic is pretty tortured though, and I
>>> don't think I'm making it better so would be happy to try cleaning it up
>>> futher once the device coherent changes are in.
>>
>> OK, can I leave this patch with you then? I have no way to test it..
>
> Yep, no worries.
>
>> Thanks,
>> Jason