Skip to content

Commit 94efde1

Browse files
johnhubbardakpm00
authored andcommitted
mm/gup: avoid an unnecessary allocation call for FOLL_LONGTERM cases
commit 53ba78d ("mm/gup: introduce check_and_migrate_movable_folios()") created a new constraint on the pin_user_pages*() API family: a potentially large internal allocation must now occur, for FOLL_LONGTERM cases. A user-visible consequence has now appeared: user space can no longer pin more than 2GB of memory anymore on x86_64. That's because, on a 4KB PAGE_SIZE system, when user space tries to (indirectly, via a device driver that calls pin_user_pages()) pin 2GB, this requires an allocation of a folio pointers array of MAX_PAGE_ORDER size, which is the limit for kmalloc(). In addition to the directly visible effect described above, there is also the problem of adding an unnecessary allocation. The **pages array argument has already been allocated, and there is no need for a redundant **folios array allocation in this case. Fix this by avoiding the new allocation entirely. This is done by referring to either the original page[i] within **pages, or to the associated folio. Thanks to David Hildenbrand for suggesting this approach and for providing the initial implementation (which I've tested and adjusted slightly) as well. [[email protected]: whitespace tweak, per David] Link: https://lkml.kernel.org/r/[email protected] [[email protected]: bypass pofs_get_folio(), per Oscar] Link: https://lkml.kernel.org/r/[email protected] Link: https://lkml.kernel.org/r/[email protected] Fixes: 53ba78d ("mm/gup: introduce check_and_migrate_movable_folios()") Signed-off-by: John Hubbard <[email protected]> Suggested-by: David Hildenbrand <[email protected]> Acked-by: David Hildenbrand <[email protected]> Reviewed-by: Oscar Salvador <[email protected]> Cc: Vivek Kasireddy <[email protected]> Cc: Dave Airlie <[email protected]> Cc: Gerd Hoffmann <[email protected]> Cc: Matthew Wilcox <[email protected]> Cc: Christoph Hellwig <[email protected]> Cc: Jason Gunthorpe <[email protected]> Cc: Peter Xu <[email protected]> Cc: Arnd Bergmann <[email protected]> Cc: Daniel Vetter <[email protected]> Cc: Dongwon Kim <[email protected]> Cc: Hugh Dickins <[email protected]> Cc: Junxiao Chang <[email protected]> Cc: <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
1 parent 247d720 commit 94efde1

File tree

1 file changed

+77
-39
lines changed

1 file changed

+77
-39
lines changed

mm/gup.c

Lines changed: 77 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -2273,20 +2273,57 @@ struct page *get_dump_page(unsigned long addr)
22732273
#endif /* CONFIG_ELF_CORE */
22742274

22752275
#ifdef CONFIG_MIGRATION
2276+
2277+
/*
2278+
* An array of either pages or folios ("pofs"). Although it may seem tempting to
2279+
* avoid this complication, by simply interpreting a list of folios as a list of
2280+
* pages, that approach won't work in the longer term, because eventually the
2281+
* layouts of struct page and struct folio will become completely different.
2282+
* Furthermore, this pof approach avoids excessive page_folio() calls.
2283+
*/
2284+
struct pages_or_folios {
2285+
union {
2286+
struct page **pages;
2287+
struct folio **folios;
2288+
void **entries;
2289+
};
2290+
bool has_folios;
2291+
long nr_entries;
2292+
};
2293+
2294+
static struct folio *pofs_get_folio(struct pages_or_folios *pofs, long i)
2295+
{
2296+
if (pofs->has_folios)
2297+
return pofs->folios[i];
2298+
return page_folio(pofs->pages[i]);
2299+
}
2300+
2301+
static void pofs_clear_entry(struct pages_or_folios *pofs, long i)
2302+
{
2303+
pofs->entries[i] = NULL;
2304+
}
2305+
2306+
static void pofs_unpin(struct pages_or_folios *pofs)
2307+
{
2308+
if (pofs->has_folios)
2309+
unpin_folios(pofs->folios, pofs->nr_entries);
2310+
else
2311+
unpin_user_pages(pofs->pages, pofs->nr_entries);
2312+
}
2313+
22762314
/*
22772315
* Returns the number of collected folios. Return value is always >= 0.
22782316
*/
22792317
static unsigned long collect_longterm_unpinnable_folios(
2280-
struct list_head *movable_folio_list,
2281-
unsigned long nr_folios,
2282-
struct folio **folios)
2318+
struct list_head *movable_folio_list,
2319+
struct pages_or_folios *pofs)
22832320
{
22842321
unsigned long i, collected = 0;
22852322
struct folio *prev_folio = NULL;
22862323
bool drain_allow = true;
22872324

2288-
for (i = 0; i < nr_folios; i++) {
2289-
struct folio *folio = folios[i];
2325+
for (i = 0; i < pofs->nr_entries; i++) {
2326+
struct folio *folio = pofs_get_folio(pofs, i);
22902327

22912328
if (folio == prev_folio)
22922329
continue;
@@ -2327,24 +2364,23 @@ static unsigned long collect_longterm_unpinnable_folios(
23272364
* Returns -EAGAIN if all folios were successfully migrated or -errno for
23282365
* failure (or partial success).
23292366
*/
2330-
static int migrate_longterm_unpinnable_folios(
2331-
struct list_head *movable_folio_list,
2332-
unsigned long nr_folios,
2333-
struct folio **folios)
2367+
static int
2368+
migrate_longterm_unpinnable_folios(struct list_head *movable_folio_list,
2369+
struct pages_or_folios *pofs)
23342370
{
23352371
int ret;
23362372
unsigned long i;
23372373

2338-
for (i = 0; i < nr_folios; i++) {
2339-
struct folio *folio = folios[i];
2374+
for (i = 0; i < pofs->nr_entries; i++) {
2375+
struct folio *folio = pofs_get_folio(pofs, i);
23402376

23412377
if (folio_is_device_coherent(folio)) {
23422378
/*
23432379
* Migration will fail if the folio is pinned, so
23442380
* convert the pin on the source folio to a normal
23452381
* reference.
23462382
*/
2347-
folios[i] = NULL;
2383+
pofs_clear_entry(pofs, i);
23482384
folio_get(folio);
23492385
gup_put_folio(folio, 1, FOLL_PIN);
23502386

@@ -2363,8 +2399,8 @@ static int migrate_longterm_unpinnable_folios(
23632399
* calling folio_isolate_lru() which takes a reference so the
23642400
* folio won't be freed if it's migrating.
23652401
*/
2366-
unpin_folio(folios[i]);
2367-
folios[i] = NULL;
2402+
unpin_folio(folio);
2403+
pofs_clear_entry(pofs, i);
23682404
}
23692405

23702406
if (!list_empty(movable_folio_list)) {
@@ -2387,12 +2423,26 @@ static int migrate_longterm_unpinnable_folios(
23872423
return -EAGAIN;
23882424

23892425
err:
2390-
unpin_folios(folios, nr_folios);
2426+
pofs_unpin(pofs);
23912427
putback_movable_pages(movable_folio_list);
23922428

23932429
return ret;
23942430
}
23952431

2432+
static long
2433+
check_and_migrate_movable_pages_or_folios(struct pages_or_folios *pofs)
2434+
{
2435+
LIST_HEAD(movable_folio_list);
2436+
unsigned long collected;
2437+
2438+
collected = collect_longterm_unpinnable_folios(&movable_folio_list,
2439+
pofs);
2440+
if (!collected)
2441+
return 0;
2442+
2443+
return migrate_longterm_unpinnable_folios(&movable_folio_list, pofs);
2444+
}
2445+
23962446
/*
23972447
* Check whether all folios are *allowed* to be pinned indefinitely (long term).
23982448
* Rather confusingly, all folios in the range are required to be pinned via
@@ -2417,16 +2467,13 @@ static int migrate_longterm_unpinnable_folios(
24172467
static long check_and_migrate_movable_folios(unsigned long nr_folios,
24182468
struct folio **folios)
24192469
{
2420-
unsigned long collected;
2421-
LIST_HEAD(movable_folio_list);
2470+
struct pages_or_folios pofs = {
2471+
.folios = folios,
2472+
.has_folios = true,
2473+
.nr_entries = nr_folios,
2474+
};
24222475

2423-
collected = collect_longterm_unpinnable_folios(&movable_folio_list,
2424-
nr_folios, folios);
2425-
if (!collected)
2426-
return 0;
2427-
2428-
return migrate_longterm_unpinnable_folios(&movable_folio_list,
2429-
nr_folios, folios);
2476+
return check_and_migrate_movable_pages_or_folios(&pofs);
24302477
}
24312478

24322479
/*
@@ -2436,22 +2483,13 @@ static long check_and_migrate_movable_folios(unsigned long nr_folios,
24362483
static long check_and_migrate_movable_pages(unsigned long nr_pages,
24372484
struct page **pages)
24382485
{
2439-
struct folio **folios;
2440-
long i, ret;
2486+
struct pages_or_folios pofs = {
2487+
.pages = pages,
2488+
.has_folios = false,
2489+
.nr_entries = nr_pages,
2490+
};
24412491

2442-
folios = kmalloc_array(nr_pages, sizeof(*folios), GFP_KERNEL);
2443-
if (!folios) {
2444-
unpin_user_pages(pages, nr_pages);
2445-
return -ENOMEM;
2446-
}
2447-
2448-
for (i = 0; i < nr_pages; i++)
2449-
folios[i] = page_folio(pages[i]);
2450-
2451-
ret = check_and_migrate_movable_folios(nr_pages, folios);
2452-
2453-
kfree(folios);
2454-
return ret;
2492+
return check_and_migrate_movable_pages_or_folios(&pofs);
24552493
}
24562494
#else
24572495
static long check_and_migrate_movable_pages(unsigned long nr_pages,

0 commit comments

Comments
 (0)