On Thu, 16 Sep 2010, Andrea Arcangeli wrote: > On Wed, Sep 15, 2010 at 04:02:24PM -0700, Hugh Dickins wrote: > > 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 > > Yes, before the fix, the page could be removed from swapcache despite > being pinned, and the cow copy could reuse the same swap entry and be > unmapped again so breaking the pte_same check. > > > 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. > > For this to happen the page would need to be removed from swapcache > and then added back to swapcache to a different swap entry. But if > that happens the "page_table" pointer would be set to a different swap > entry too, so failing the pte_same check. If the swap entry of the > page changes the page_table will change with it, so the pte_same check > will fail in the first place, so at first glance it looks like > checking the page_private isn't necessary and the pte_same check on > the page_table is enough. I agree that if my scenario happened on its own, the pte_same check would catch it. But if my scenario happens along with your scenario (and I'm thinking that the combination is not that much less likely than either alone), then the PageSwapCache test will succeed and the pte_same test will succeed, but we're still putting the wrong page into the pte, since this page is now represented by a different swap entry (and the page that should be there by our original swap entry). Hugh -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxxx For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>