kevin/linux
1
Fork 0
Code

mm/gup: stop leaking pinned pages in low memory conditions

Browse Source 
If a driver tries to call any of the pin_user_pages*(FOLL_LONGTERM) family
of functions, and requests "too many" pages, then the call will
erroneously leave pages pinned.  This is visible in user space as an
actual memory leak.

Repro is trivial: just make enough pin_user_pages(FOLL_LONGTERM) calls to
exhaust memory.

The root cause of the problem is this sequence, within
__gup_longterm_locked():

    __get_user_pages_locked()
    rc = check_and_migrate_movable_pages()

...which gets retried in a loop.  The loop error handling is incomplete,
clearly due to a somewhat unusual and complicated tri-state error API. 
But anyway, if -ENOMEM, or in fact, any unexpected error is returned from
check_and_migrate_movable_pages(), then __gup_longterm_locked() happily
returns the error, while leaving the pages pinned.

In the failed case, which is an app that requests (via a device driver)
30720000000 bytes to be pinned, and then exits, I see this:

    $ grep foll /proc/vmstat
        nr_foll_pin_acquired 7502048
        nr_foll_pin_released 2048

And after applying this patch, it returns to balanced pins:

    $ grep foll /proc/vmstat
        nr_foll_pin_acquired 7502048
        nr_foll_pin_released 7502048

Note that the child routine, check_and_migrate_movable_folios(), avoids
this problem, by unpinning any folios in the **folios argument, before
returning an error.

Fix this by making check_and_migrate_movable_pages() behave in exactly the
same way as check_and_migrate_movable_folios(): unpin all pages in
**pages, before returning an error.

Also, documentation was an aggravating factor, so:

1) Consolidate the documentation for these two routines, now that they
have identical external behavior.

2) Rewrite the consolidated documentation:

    a) Clearly list the three return code cases, and what happens in
    each case.

    b) Mention that one of the cases unpins the pages or folios, before
    returning an error code.

Link: https://lkml.kernel.org/r/20241018223411.310331-1-jhubbard@nvidia.com
Fixes: 24a95998e9 ("mm/gup.c: simplify and fix check_and_migrate_movable_pages() return codes")
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
Reviewed-by: Alistair Popple <apopple@nvidia.com>
Suggested-by: David Hildenbrand <david@redhat.com>
Cc: Shigeru Yoshida <syoshida@redhat.com>
Cc: Jason Gunthorpe <jgg@nvidia.com>
Cc: Minchan Kim <minchan@kernel.org>
Cc: Pasha Tatashin <pasha.tatashin@soleen.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
This commit is contained in:
John Hubbard 2024-10-18 15:34:11 -07:00 committed by Andrew Morton
parent 01626a1823
commit aa6f8b2593
1 changed files with 19 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
mm/gup.c
View File

@ -2394,20 +2394,25 @@ err:
} }
/* /*
* Check whether all folios are *allowed* to be pinned indefinitely (longterm). * Check whether all folios are *allowed* to be pinned indefinitely (long term).
* Rather confusingly, all folios in the range are required to be pinned via * Rather confusingly, all folios in the range are required to be pinned via
* FOLL_PIN, before calling this routine. * FOLL_PIN, before calling this routine.
* *
* If any folios in the range are not allowed to be pinned, then this routine * Return values:
* will migrate those folios away, unpin all the folios in the range and return
* -EAGAIN. The caller should re-pin the entire range with FOLL_PIN and then
* call this routine again.
* *
* If an error other than -EAGAIN occurs, this indicates a migration failure. * 0: if everything is OK and all folios in the range are allowed to be pinned,
* The caller should give up, and propagate the error back up the call stack.
*
* If everything is OK and all folios in the range are allowed to be pinned,
* then this routine leaves all folios pinned and returns zero for success. * then this routine leaves all folios pinned and returns zero for success.
*
* -EAGAIN: if any folios in the range are not allowed to be pinned, then this
* routine will migrate those folios away, unpin all the folios in the range. If
* migration of the entire set of folios succeeds, then -EAGAIN is returned. The
* caller should re-pin the entire range with FOLL_PIN and then call this
* routine again.
*
* -ENOMEM, or any other -errno: if an error *other* than -EAGAIN occurs, this
* indicates a migration failure. The caller should give up, and propagate the
* error back up the call stack. The caller does not need to unpin any folios in
* that case, because this routine will do the unpinning.
*/ */
static long check_and_migrate_movable_folios(unsigned long nr_folios, static long check_and_migrate_movable_folios(unsigned long nr_folios,
struct folio **folios) struct folio **folios)
@ -2425,10 +2430,8 @@ static long check_and_migrate_movable_folios(unsigned long nr_folios,
} }
/* /*
* This routine just converts all the pages in the @pages array to folios and * Return values and behavior are the same as those for
* calls check_and_migrate_movable_folios() to do the heavy lifting. * check_and_migrate_movable_folios().
*
* Please see the check_and_migrate_movable_folios() documentation for details.
*/ */
static long check_and_migrate_movable_pages(unsigned long nr_pages, static long check_and_migrate_movable_pages(unsigned long nr_pages,
struct page **pages) struct page **pages)
@ -2437,8 +2440,10 @@ static long check_and_migrate_movable_pages(unsigned long nr_pages,
long i, ret; long i, ret;
folios = kmalloc_array(nr_pages, sizeof(*folios), GFP_KERNEL); folios = kmalloc_array(nr_pages, sizeof(*folios), GFP_KERNEL);
if (!folios) if (!folios) {
unpin_user_pages(pages, nr_pages);
return -ENOMEM; return -ENOMEM;
}
for (i = 0; i < nr_pages; i++) for (i = 0; i < nr_pages; i++)
folios[i] = page_folio(pages[i]); folios[i] = page_folio(pages[i]);