Skip to content

Commit

Permalink
mm: try_to_unuse check removing right swap
Browse files Browse the repository at this point in the history
There's a possible race in try_to_unuse() which Nick Piggin led me to two
years ago.  Where it does lock_page() after read_swap_cache_async(), what
if another task removed that page from swapcache just before we locked it?

It would sail though the (*swap_map > 1) tests doing nothing (because it
could not have been removed from swapcache before its swap references were
gone), until it reaches the delete_from_swap_cache(page) near the bottom.

Now imagine that this page has been allocated to swap on a different swap
area while we dropped page lock (perhaps at the top, perhaps in unuse_mm):
we could wrongly remove from swap cache before the page has been written
to swap, so a subsequent do_swap_page() would read in stale data from
swap.

I think this case could not happen before: remove_exclusive_swap_page()
refused while page count was raised.  But now with reuse_swap_page() and
try_to_free_swap() removing from swap cache without minding page count, I
think it could happen - the previous patch argued that it was safe because
try_to_unuse() already ignored page count, but overlooked that it might be
breaking the assumptions in try_to_unuse() itself.

Signed-off-by: Hugh Dickins <[email protected]>
Cc: Lee Schermerhorn <[email protected]>
Cc: Rik van Riel <[email protected]>
Cc: Nick Piggin <[email protected]>
Cc: KAMEZAWA Hiroyuki <[email protected]>
Cc: Robin Holt <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
  • Loading branch information
Hugh Dickins authored and torvalds committed Jan 6, 2009
1 parent a2c43ee commit 68bdc8d
Showing 1 changed file with 10 additions and 1 deletion.
11 changes: 10 additions & 1 deletion mm/swapfile.c
Original file line number Diff line number Diff line change
Expand Up @@ -889,7 +889,16 @@ static int try_to_unuse(unsigned int type)
lock_page(page);
wait_on_page_writeback(page);
}
if (PageSwapCache(page))

/*
* It is conceivable that a racing task removed this page from
* swap cache just before we acquired the page lock at the top,
* or while we dropped it in unuse_mm(). The page might even
* be back in swap cache on another swap area: that we must not
* delete, since it may not have been written out to swap yet.
*/
if (PageSwapCache(page) &&
likely(page_private(page) == entry.val))
delete_from_swap_cache(page);

/*
Expand Down

0 comments on commit 68bdc8d

Please sign in to comment.