Skip to content

Commit 87c460a

Browse files
Hugh Dickinstorvalds
Hugh Dickins
authored andcommitted
mm/khugepaged: collapse_shmem() without freezing new_page
khugepaged's collapse_shmem() does almost all of its work, to assemble the huge new_page from 512 scattered old pages, with the new_page's refcount frozen to 0 (and refcounts of all old pages so far also frozen to 0). Including shmem_getpage() to read in any which were out on swap, memory reclaim if necessary to allocate their intermediate pages, and copying over all the data from old to new. Imagine the frozen refcount as a spinlock held, but without any lock debugging to highlight the abuse: it's not good, and under serious load heads into lockups - speculative getters of the page are not expecting to spin while khugepaged is rescheduled. One can get a little further under load by hacking around elsewhere; but fortunately, freezing the new_page turns out to have been entirely unnecessary, with no hacks needed elsewhere. The huge new_page lock is already held throughout, and guards all its subpages as they are brought one by one into the page cache tree; and anything reading the data in that page, without the lock, before it has been marked PageUptodate, would already be in the wrong. So simply eliminate the freezing of the new_page. Each of the old pages remains frozen with refcount 0 after it has been replaced by a new_page subpage in the page cache tree, until they are all unfrozen on success or failure: just as before. They could be unfrozen sooner, but cause no problem once no longer visible to find_get_entry(), filemap_map_pages() and other speculative lookups. Link: http://lkml.kernel.org/r/[email protected] Fixes: f3f0e1d ("khugepaged: add support of collapse for tmpfs/shmem pages") Signed-off-by: Hugh Dickins <[email protected]> Acked-by: Kirill A. Shutemov <[email protected]> Cc: Jerome Glisse <[email protected]> Cc: Konstantin Khlebnikov <[email protected]> Cc: Matthew Wilcox <[email protected]> Cc: <[email protected]> [4.8+] Signed-off-by: Andrew Morton <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
1 parent 042a308 commit 87c460a

File tree

1 file changed

+7
-12
lines changed

1 file changed

+7
-12
lines changed

mm/khugepaged.c

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1287,19 +1287,19 @@ static void retract_page_tables(struct address_space *mapping, pgoff_t pgoff)
12871287
* collapse_shmem - collapse small tmpfs/shmem pages into huge one.
12881288
*
12891289
* Basic scheme is simple, details are more complex:
1290-
* - allocate and freeze a new huge page;
1290+
* - allocate and lock a new huge page;
12911291
* - scan page cache replacing old pages with the new one
12921292
* + swap in pages if necessary;
12931293
* + fill in gaps;
12941294
* + keep old pages around in case rollback is required;
12951295
* - if replacing succeeds:
12961296
* + copy data over;
12971297
* + free old pages;
1298-
* + unfreeze huge page;
1298+
* + unlock huge page;
12991299
* - if replacing failed;
13001300
* + put all pages back and unfreeze them;
13011301
* + restore gaps in the page cache;
1302-
* + free huge page;
1302+
* + unlock and free huge page;
13031303
*/
13041304
static void collapse_shmem(struct mm_struct *mm,
13051305
struct address_space *mapping, pgoff_t start,
@@ -1333,13 +1333,11 @@ static void collapse_shmem(struct mm_struct *mm,
13331333
__SetPageSwapBacked(new_page);
13341334
new_page->index = start;
13351335
new_page->mapping = mapping;
1336-
BUG_ON(!page_ref_freeze(new_page, 1));
13371336

13381337
/*
1339-
* At this point the new_page is 'frozen' (page_count() is zero),
1340-
* locked and not up-to-date. It's safe to insert it into the page
1341-
* cache, because nobody would be able to map it or use it in other
1342-
* way until we unfreeze it.
1338+
* At this point the new_page is locked and not up-to-date.
1339+
* It's safe to insert it into the page cache, because nobody would
1340+
* be able to map it or use it in another way until we unlock it.
13431341
*/
13441342

13451343
/* This will be less messy when we use multi-index entries */
@@ -1491,9 +1489,8 @@ static void collapse_shmem(struct mm_struct *mm,
14911489
index++;
14921490
}
14931491

1494-
/* Everything is ready, let's unfreeze the new_page */
14951492
SetPageUptodate(new_page);
1496-
page_ref_unfreeze(new_page, HPAGE_PMD_NR);
1493+
page_ref_add(new_page, HPAGE_PMD_NR - 1);
14971494
set_page_dirty(new_page);
14981495
mem_cgroup_commit_charge(new_page, memcg, false, true);
14991496
lru_cache_add_anon(new_page);
@@ -1541,8 +1538,6 @@ static void collapse_shmem(struct mm_struct *mm,
15411538
VM_BUG_ON(nr_none);
15421539
xas_unlock_irq(&xas);
15431540

1544-
/* Unfreeze new_page, caller would take care about freeing it */
1545-
page_ref_unfreeze(new_page, 1);
15461541
mem_cgroup_cancel_charge(new_page, memcg, true);
15471542
new_page->mapping = NULL;
15481543
}

0 commit comments

Comments
 (0)