Re: s390's PageSwapCache test

[Hugh Dickins <hugh@veritas.com>]

On Sat, 2 Aug 2008, Martin Schwidefsky wrote:
> 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).
> ... 
> 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.
                       exits
> 
> 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?

Yes, it took me a while to understand, but you are right.  I was
blinkered, thinking always of page_remove_rmap called by zap_pte_range,
forgetting page_remove_rmap called by try_to_unmap_one i.e. vmscan.

Your example is dealt with on the non-s390 arches by try_to_unmap_one's
	if (pte_dirty(pteval))
		set_page_dirty(page);
but s390 needs something else: and since you can't do it until the
last mapping is removed, you're stuck with detecting the possibility
of this case by testing PageSwapCache in there.

It's a pity that so often it's irrelevant, but I can't offhand think
of a better answer, and it would only be a small optimization to an
already non-optimal path.

Good thinking, Martin, and thank you for enlightening me:
when I'm next in patch mode I'll add a comment there on it.

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>