linux-kernel - Re: [RFC PATCH] s390: mm: rmap: Transfer storage key to struct page under the page lock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20120417150237.0abb8ec5@de.ibm.com>
Date:	Tue, 17 Apr 2012 15:02:37 +0200
From:	Martin Schwidefsky <schwidefsky@...ibm.com>
To:	Mel Gorman <mgorman@...e.de>
Cc:	Heiko Carstens <heiko.carstens@...ibm.com>,
	Hugh Dickins <hughd@...gle.com>,
	Rik van Riel <riel@...hat.com>,
	Linux-MM <linux-mm@...ck.org>,
	Linux-S390 <linux-s390@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH] s390: mm: rmap: Transfer storage key to struct page
 under the page lock

On Tue, 17 Apr 2012 13:29:25 +0100
Mel Gorman <mgorman@...e.de> wrote:

> On Mon, Apr 16, 2012 at 05:50:40PM +0200, Martin Schwidefsky wrote:
> > On Mon, 16 Apr 2012 15:14:23 +0100
> > Mel Gorman <mgorman@...e.de> wrote:
> > 
> > > This patch is horribly ugly and there has to be a better way of doing
> > > it. I'm looking for suggestions on what s390 can do here that is not
> > > painful or broken. 
> > > 
> > > However, s390 needs a better way of guarding against
> > > PageSwapCache pages being removed from the radix tree while set_page_dirty()
> > > is being called. The patch would be marginally better if in the PageSwapCache
> > > case we simply tried to lock once and in the contended case just fail to
> > > propogate the storage key. I lack familiarity with the s390 architecture
> > > to be certain if this is safe or not. Suggestions on a better fix?
> > 
> > One though that crossed my mind is that maybe a better approach would be
> > to move the page_test_and_clear_dirty check out of page_remove_rmap.
> > What we need to look out for are code sequences of the form:
> > 
> > 	if (pte_dirty(pte))
> > 		set_page_dirty(page);
> > 	...
> > 	page_remove_rmap(page);
> > 
> > There are four of those as far as I can see: in try_to_unmap_one,
> > try_to_unmap_cluster, zap_pte, and zap_pte_range.
> > 
> > A valid implementation for s390 would be to test and clear the changed
> > bit in the storage key for every of those pte_dirty() calls.
> > 
> > 	if (pte_dirty(pte) || page_test_and_clear_dirty(page))
> > 		set_page_dirty(page);
> > 	...
> > 	page_remove_rmap(page); /* w/o page_test_clear_dirty */
> > 
> 
> In the zap_pte_range() case at least, pte_dirty() is only being checked
> for !PageAnon pages so if we took this approach we would miss
> PageSwapCache pages. If we added the check then the same problem is hit
> and we'd need additional logic there for s390 to drop the PTL, take the
> page lock and retry the operation. It'd still be ugly :(

Well if x86 can get away with ignoring PageSwapCache pages in zap_pte_range()
pages then s390 should be able to get away with it as well, no ?

-- 
blue skies,
   Martin.

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/