Skip to content

Commit

Permalink
mm: migrate: don't rely on __PageMovable() of newpage after unlocking it
Browse files Browse the repository at this point in the history
We had a race in the old balloon compaction code before b1123ea
("mm: balloon: use general non-lru movable page feature") refactored it
that became visible after backporting 195a8c4 ("virtio-balloon:
deflate via a page list") without the refactoring.

The bug existed from commit d6d86c0 ("mm/balloon_compaction:
redesign ballooned pages management") till b1123ea ("mm: balloon:
use general non-lru movable page feature").  d6d86c0
("mm/balloon_compaction: redesign ballooned pages management") was
backported to 3.12, so the broken kernels are stable kernels [3.12 -
4.7].

There was a subtle race between dropping the page lock of the newpage in
__unmap_and_move() and checking for __is_movable_balloon_page(newpage).

Just after dropping this page lock, virtio-balloon could go ahead and
deflate the newpage, effectively dequeueing it and clearing PageBalloon,
in turn making __is_movable_balloon_page(newpage) fail.

This resulted in dropping the reference of the newpage via
putback_lru_page(newpage) instead of put_page(newpage), leading to
page->lru getting modified and a !LRU page ending up in the LRU lists.
With 195a8c4 ("virtio-balloon: deflate via a page list")
backported, one would suddenly get corrupted lists in
release_pages_balloon():

- WARNING: CPU: 13 PID: 6586 at lib/list_debug.c:59 __list_del_entry+0xa1/0xd0
- list_del corruption. prev->next should be ffffe253961090a0, but was dead000000000100

Nowadays this race is no longer possible, but it is hidden behind very
ugly handling of __ClearPageMovable() and __PageMovable().

__ClearPageMovable() will not make __PageMovable() fail, only
PageMovable().  So the new check (__PageMovable(newpage)) will still
hold even after newpage was dequeued by virtio-balloon.

If anybody would ever change that special handling, the BUG would be
introduced again.  So instead, make it explicit and use the information
of the original isolated page before migration.

This patch can be backported fairly easy to stable kernels (in contrast
to the refactoring).

Link: http://lkml.kernel.org/r/[email protected]
Fixes: d6d86c0 ("mm/balloon_compaction: redesign ballooned pages management")
Signed-off-by: David Hildenbrand <[email protected]>
Reported-by: Vratislav Bendel <[email protected]>
Acked-by: Michal Hocko <[email protected]>
Acked-by: Rafael Aquini <[email protected]>
Cc: Mel Gorman <[email protected]>
Cc: "Kirill A. Shutemov" <[email protected]>
Cc: Michal Hocko <[email protected]>
Cc: Naoya Horiguchi <[email protected]>
Cc: Jan Kara <[email protected]>
Cc: Andrea Arcangeli <[email protected]>
Cc: Dominik Brodowski <[email protected]>
Cc: Matthew Wilcox <[email protected]>
Cc: Vratislav Bendel <[email protected]>
Cc: Rafael Aquini <[email protected]>
Cc: Konstantin Khlebnikov <[email protected]>
Cc: Minchan Kim <[email protected]>
Cc: <[email protected]>	[3.12 - 4.7]
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
  • Loading branch information
davidhildenbrand authored and torvalds committed Feb 1, 2019
1 parent 7b2489d commit e0a352f
Showing 1 changed file with 5 additions and 2 deletions.
7 changes: 5 additions & 2 deletions mm/migrate.c
Original file line number Diff line number Diff line change
Expand Up @@ -1130,10 +1130,13 @@ static int __unmap_and_move(struct page *page, struct page *newpage,
* If migration is successful, decrease refcount of the newpage
* which will not free the page because new page owner increased
* refcounter. As well, if it is LRU page, add the page to LRU
* list in here.
* list in here. Use the old state of the isolated source page to
* determine if we migrated a LRU page. newpage was already unlocked
* and possibly modified by its owner - don't rely on the page
* state.
*/
if (rc == MIGRATEPAGE_SUCCESS) {
if (unlikely(__PageMovable(newpage)))
if (unlikely(!is_lru))
put_page(newpage);
else
putback_lru_page(newpage);
Expand Down

0 comments on commit e0a352f

Please sign in to comment.