Re: [PATCH v2 1/2] mm/gup: stop leaking pinned pages in low memory conditions

[John Hubbard <jhubbard@nvidia.com>]

On 10/20/24 3:59 PM, Alistair Popple wrote:
> John Hubbard <jhubbard@nvidia.com> writes:
>> On 10/18/24 12:47 AM, David Hildenbrand wrote:
>>> On 18.10.24 03:17, John Hubbard wrote:
> [...]
>> And actually this whole thing of "pin the pages, just for a short time, even
>> though you're not allowed to" is partly why this area is so entertaining.
> 
> I'm looking at your v3 but as an aside I disagree with this
> statement. AFAIK you're always allowed to pin the pages for a short time
> (ie. !FOLL_LONGTERM), or did I misunderstand your comment?

Sort of: short term pins are allowed, but at this point in the code,
here:

pin_user_pages(FOLL_PIN | FOLL_LONGTERM)
     __gup_longterm_locked()
          __get_user_pages_locked(FOLL_PIN | FOLL_LONGTERM)

, just before calling check_and_migrate_movable_pages(), we have already
filtered out any cases other than (FOLL_PIN | FOLL_LONGTERM).

And that means that code has taken a *longterm* pin of presumably short
duration (this incongruity bothers me), on pages that are not actually
allowed to be long term pinned. That also feels imperfect, even though
it is supposedly short duration...except that page migration is only
sort of short...hmmm.

I'm starting to think that migrating any ZONE_MOVABLE pages away first
might be better.

Since I'm already preparing that "wait for folio refcount" idea for
migration, which is almost related, I'll take a closer look at this
idea while I'm at it.


thanks,
-- 
John Hubbard