Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934493AbYCDXax (ORCPT ); Tue, 4 Mar 2008 18:30:53 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S935308AbYCDXZn (ORCPT ); Tue, 4 Mar 2008 18:25:43 -0500 Received: from bombadil.infradead.org ([18.85.46.34]:35335 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935001AbYCDXZh (ORCPT ); Tue, 4 Mar 2008 18:25:37 -0500 Subject: Re: [RFC] Notifier for Externally Mapped Memory (EMM) From: Peter Zijlstra To: Christoph Lameter Cc: Andrea Arcangeli , Jack Steiner , Nick Piggin , akpm@linux-foundation.org, Robin Holt , Avi Kivity , kvm-devel@lists.sourceforge.net, general@lists.openfabrics.org, Steve Wise , Roland Dreier , Kanoj Sarcar , linux-kernel@vger.kernel.org, linux-mm@kvack.org, daniel.blueman@quadrics.com In-Reply-To: References: <20080221144023.GC9427@v2.random> <20080221161028.GA14220@sgi.com> <20080227192610.GF28483@v2.random> <20080302155457.GK8091@v2.random> <20080303213707.GA8091@v2.random> <20080303220502.GA5301@v2.random> <47CC9B57.5050402@qumranet.com> <20080304133020.GC5301@v2.random> <20080304222030.GB8951@v2.random> <1204670529.6241.52.camel@lappy> Content-Type: text/plain Date: Wed, 05 Mar 2008 00:25:00 +0100 Message-Id: <1204673100.6241.59.camel@lappy> Mime-Version: 1.0 X-Mailer: Evolution 2.21.90 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2590 Lines: 78 On Tue, 2008-03-04 at 15:14 -0800, Christoph Lameter wrote: > On Tue, 4 Mar 2008, Peter Zijlstra wrote: > > > > > On Tue, 2008-03-04 at 14:35 -0800, Christoph Lameter wrote: > > > > > RCU means that the callbacks occur in an atomic context. > > > > Not really, if it requires moving the VM locks to sleepable locks under > > a .config option, I think its also fair to require PREEMPT_RCU. > > Which would make the patchset pretty complex. RCU is not needed with a > single linked list. Linked list operations can exploit atomic pointer > updates and we only tear down the list when a single execution thread > remains. OK, that constraint on removal makes it work. > Having said that: Here a couple of updates to address Andrea's complaint > that we not check the referenced bit from the external mapper when the > rerferences bit is set on an OS pte. > > Plus two barriers to ensure that a new emm notifier object becomes > visible before the base pointer is updated. > > Signed-off-by: Christoph Lameter > > --- > mm/rmap.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > Index: linux-2.6/mm/rmap.c > =================================================================== > --- linux-2.6.orig/mm/rmap.c 2008-03-04 14:36:36.321922321 -0800 > +++ linux-2.6/mm/rmap.c 2008-03-04 15:10:46.159429369 -0800 > @@ -298,10 +298,10 @@ static int page_referenced_one(struct pa > > (*mapcount)--; > pte_unmap_unlock(pte, ptl); > - if (!referenced) > - /* rmap lock held */ > - referenced = emm_notify(mm, emm_referenced, > - address, address + PAGE_SIZE); > + > + /* rmap lock held */ > + if (emm_notify(mm, emm_referenced, address, address + PAGE_SIZE)) > + referenced = 1; referenced++; seems more in-style with the rest of that code.. > out: > return referenced; > } > @@ -1057,6 +1057,7 @@ EXPORT_SYMBOL_GPL(emm_notifier_release); > void emm_notifier_register(struct emm_notifier *e, struct mm_struct *mm) > { > e->next = mm->emm_notifier; > + smp_wmb(); > mm->emm_notifier = e; > } > EXPORT_SYMBOL_GPL(emm_notifier_register); > @@ -1069,6 +1070,7 @@ int __emm_notify(struct mm_struct *mm, e > int x; > > while (e) { > + smp_rmb(); > if (e->func) { > x = e->func(e, mm, op, start, end); > if (x) We generally require comments around barriers.. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/