Re: s390's PageSwapCache test

[Martin Schwidefsky <schwidefsky@de.ibm.com>]

Hi Hugh,

On Sat, 2008-08-02 at 10:05 +0100, Hugh Dickins wrote:
> I'm slightly bothered by that PageSwapCache() test you've just added
> in page_remove_rmap(), before s390's page_test_dirty():
> 
> 		if ((!PageAnon(page) || PageSwapCache(page)) &&
> 		    page_test_dirty(page)) {
> 			page_clear_dirty(page);
> 			set_page_dirty(page);
> 		}
> 
> It's not wrong; but if it's necessary, then I need to understand why;
> and if it's unnecessary, then we'd do better to remove it (optimizing
> your optimization a little).

I want to play safe. I can conclude that the page dirty bit is of no
interest if the page is a purely anonymous page without a backing. That
is what the test checks for.

> I believe it's unnecessary: it is possible, yes, to arrive here and
> find the anon page dirty with respect to what's on swap disk; but
> because anon pages are COWed, never sharing modification with other
> users, that will only be so if we're the only user of that page, and
> about to free it, in which case no point in doing the set_page_dirty().

Hmm, what about the following sequence:
1) a page is added to the swap
2) the page is dirtied again
3) the process forks
4) the first process does an mlock
5) vmscan succeeds in replacing the pte of the second process with a
swap entry but fails for the pte of the first process.
6) the first process exists.

If the PageSwapCache() check is missing zap_pte_range() will drop the
last pte for the page but won't transfer the dirty bit.
Wouldn't that break?

> For a very similar case, see the PageAnon() test in zap_pte_range(),
> where we also skip the set_page_dirty().

-- 
blue skies,
  Martin.

"Reality continues to ruin my life." - Calvin.


--
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>