On Wed, 2 Jul 2014, Liu Ping Fan wrote: > There is race between do_swap_page() and swap_writepage(), if > do_swap_page() had deleted a page from swap cache, there is no need > to write it. So changing the ret of try_to_free_swap() to make > swap_writepage() aware of this scene. Is this an inefficiency that you have noticed in practice, or something that you think you spotted by code inspection? I don't see how it can happen: all the places I know of that call swap_writepage() (including vmscan.c's mapping->a_ops->writepage) have not dropped page lock since setting or checking PageSwapCache, and page lock is supposed to protect against deletion from swap cache. Has that changed? Please point out where. > > Signed-off-by: Liu Ping Fan <pingfank@xxxxxxxxxxxxxxxxxx> > --- > mm/swapfile.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/swapfile.c b/mm/swapfile.c > index 4c524f7..9d80671 100644 > --- a/mm/swapfile.c > +++ b/mm/swapfile.c > @@ -910,7 +910,7 @@ int try_to_free_swap(struct page *page) > VM_BUG_ON_PAGE(!PageLocked(page), page); > > if (!PageSwapCache(page)) > - return 0; > + return -1; Previously it returned either 0 or 1, which is what __try_to_reclaim_swap() says it returns; so better to stick to 0 or 1, unless you have good reason to add a distinct value. It's true that by the time __try_to_reclaim_swap() has got the page lock, the page might have been removed from swap cache, and we could then treat that as swap_was_freed (even though it was not freed by the caller). But it's a very narrow window, and no great advantage to do so: I don't think it's worth changing try_to_free_swap() semantics for, but you could persuade us. Hugh > if (PageWriteback(page)) > return 0; > if (page_swapcount(page)) > -- > 1.8.1.4 -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>