linux-kernel - Re: [PATCH v8 1/5] mm: introduce a common interface for balloon pages mobility

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]

Date:	Tue, 21 Aug 2012 14:42:52 -0300
From:	Rafael Aquini <aquini@...hat.com>
To:	"Michael S. Tsirkin" <mst@...hat.com>
Cc:	Peter Zijlstra <peterz@...radead.org>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org,
	virtualization@...ts.linux-foundation.org,
	Rusty Russell <rusty@...tcorp.com.au>,
	Rik van Riel <riel@...hat.com>, Mel Gorman <mel@....ul.ie>,
	Andi Kleen <andi@...stfloor.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
	Minchan Kim <minchan@...nel.org>
Subject: Re: [PATCH v8 1/5] mm: introduce a common interface for balloon
 pages mobility

On Tue, Aug 21, 2012 at 06:41:42PM +0300, Michael S. Tsirkin wrote:
> On Tue, Aug 21, 2012 at 05:16:06PM +0200, Peter Zijlstra wrote:
> > On Tue, 2012-08-21 at 16:52 +0300, Michael S. Tsirkin wrote:
> > > > +             rcu_read_lock();
> > > > +             mapping = rcu_dereference(page->mapping);
> > > > +             if (mapping_balloon(mapping))
> > > > +                     ret = true;
> > > > +             rcu_read_unlock();
> > > 
> > > This looks suspicious: you drop rcu_read_unlock
> > > so can't page switch from balloon to non balloon? 
> > 
> > RCU read lock is a non-exclusive lock, it cannot avoid anything like
> > that.
> 
> You are right, of course. So even keeping rcu_read_lock across both test
> and operation won't be enough - you need to make this function return
> the mapping and pass it to isolate_page/putback_page so that it is only
> dereferenced once.
>
No, I need to dereference page->mapping to check ->mapping flags here, before
returning. Remember this function is used at MM's compaction/migration inner
circles to identify ballooned pages and decide what's the next step. This
function is doing the right thing, IMHO.

Also, looking at how compaction/migration work, we verify the only critical path
for this function is the page isolation step. The other steps (migration and
putback) perform their work on private lists previouly isolated from a given
source.

So, we just need to make sure that the isolation part does not screw things up
by isolating pages that balloon driver is about to release. That's why there are
so many checkpoints down the page isolation path assuring we really are
isolating a balloon page. 


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