Re: [PATCH] fix swapin race condition

[Hugh Dickins <hughd@google.com>]

On Sun, 5 Sep 2010, Hugh Dickins wrote:
> On Fri, 3 Sep 2010, Andrea Arcangeli wrote:
> > From: Andrea Arcangeli <aarcange@redhat.com>
> > 
> > The pte_same check is reliable only if the swap entry remains pinned
> > (by the page lock on swapcache). We've also to ensure the swapcache
> > isn't removed before we take the lock as try_to_free_swap won't care
> > about the page pin.
> > 
> > Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
> 
> Acked-by: Hugh Dickins <hughd@google.com>
> 
> Yes, it's a great little find, and long predates the KSM hooks you've
> had to adjust.  It does upset me (aesthetically) that the KSM case now
> intrudes into do_swap_swap() much more than it used to; but I have not
> come up with a better solution, so yes, let's go forward with this.

I had an afterthought, something I've not thought through fully, but am
reminded of by Greg's mail for stable: is your patch incomplete?  Just
as it's very unlikely but conceivable the pte_same() test is inadequate,
isn't the PageSwapCache() test you've added to do_swap_page() inadequate?
Doesn't it need a "page_private(page) == entry.val" test too?

Just as it's conceivable that the same swap has got reused (either via
try_to_free_swap or via swapoff+swapon) for a COWed version of the page
in that pte slot meanwhile, isn't it conceivable that the page we hold
while waiting for pagelock, has got freed from swap then reallocated to
elsewhere on swap meanwhile?  Which, together with your scenario (and I
suspect the two unlikelihoods are not actually to be multiplied), would
still lead to the wrong result, unless we add the further test.

I apologize if I'm holding up your stable fix, and just vying against
you in a "world's most unlikely VM race" competition, but I don't at
present see what prevents this variant.

Hugh

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>