From mboxrd@z Thu Jan 1 00:00:00 1970 From: Oleg Nesterov Subject: Re: [PATCH 00/21] mm: Preemptibility -v6 Date: Mon, 24 Jan 2011 15:34:07 +0100 Message-ID: <20110124143407.GA19856@redhat.com> References: <20101126143843.801484792@chello.nl> <1295457039.28776.137.camel@laptop> <1295624034.28776.303.camel@laptop> <1295871714.28776.406.camel@laptop> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <1295871714.28776.406.camel@laptop> Sender: owner-linux-mm@kvack.org To: Peter Zijlstra Cc: Hugh Dickins , Andrew Morton , Benjamin Herrenschmidt , David Miller , Nick Piggin , Martin Schwidefsky , linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org, linux-mm@kvack.org, Andrea Arcangeli , "Paul E. McKenney" List-Id: linux-arch.vger.kernel.org On 01/24, Peter Zijlstra wrote: > > On Fri, 2011-01-21 at 16:33 +0100, Peter Zijlstra wrote: > > > Index: linux-2.6/mm/rmap.c > > =================================================================== > > --- linux-2.6.orig/mm/rmap.c > > +++ linux-2.6/mm/rmap.c > > @@ -1559,9 +1559,20 @@ void __put_anon_vma(struct anon_vma *ano > > * Synchronize against page_lock_anon_vma() such that > > * we can safely hold the lock without the anon_vma getting > > * freed. > > + * > > + * Relies on the full mb implied by the atomic_dec_and_test() from > > + * put_anon_vma() against the full mb implied by mutex_trylock() from > > + * page_lock_anon_vma(). This orders: > > + * > > + * page_lock_anon_vma() VS put_anon_vma() > > + * mutex_trylock() atomic_dec_and_test() > > + * smp_mb() smp_mb() > > + * atomic_read() mutex_is_locked() > > Bah!, I thought all mutex_trylock() implementations used an atomic op > with return value (which implies a mb), but it looks like (at least*) > PPC doesn't and only provides a LOCK barrier. But, mutex_trylock() must imply the one-way barrier, otherwise it is buggy, no? IOW, page_lock_anon_vma() does: if (mutex_trylock(anon_vma->root->lock)) { ... atomic_read(&anon_vma->refcount); ... } If this atomic_read() can leak out of the critical section, then I think mutex_trylock() should be fixed. Or I misunderstood the problem completely... BTW, from https://lkml.org/lkml/2010/11/26/213 + * Similar to page_get_anon_vma() except it locks the anon_vma. ... - struct anon_vma *anon_vma = page_get_anon_vma(page); looks like, page_get_anon_vma() becomes unused. Oleg. -- 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/ . Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/ Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com ([209.132.183.28]:15253 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752899Ab1AXOms (ORCPT ); Mon, 24 Jan 2011 09:42:48 -0500 Date: Mon, 24 Jan 2011 15:34:07 +0100 From: Oleg Nesterov Subject: Re: [PATCH 00/21] mm: Preemptibility -v6 Message-ID: <20110124143407.GA19856@redhat.com> References: <20101126143843.801484792@chello.nl> <1295457039.28776.137.camel@laptop> <1295624034.28776.303.camel@laptop> <1295871714.28776.406.camel@laptop> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1295871714.28776.406.camel@laptop> Sender: linux-arch-owner@vger.kernel.org List-ID: To: Peter Zijlstra Cc: Hugh Dickins , Andrew Morton , Benjamin Herrenschmidt , David Miller , Nick Piggin , Martin Schwidefsky , linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org, linux-mm@kvack.org, Andrea Arcangeli , "Paul E. McKenney" Message-ID: <20110124143407.bekzW9bu1qvfgrFQa81siEcvW9aqUx2tEiUZqZLJzoo@z> On 01/24, Peter Zijlstra wrote: > > On Fri, 2011-01-21 at 16:33 +0100, Peter Zijlstra wrote: > > > Index: linux-2.6/mm/rmap.c > > =================================================================== > > --- linux-2.6.orig/mm/rmap.c > > +++ linux-2.6/mm/rmap.c > > @@ -1559,9 +1559,20 @@ void __put_anon_vma(struct anon_vma *ano > > * Synchronize against page_lock_anon_vma() such that > > * we can safely hold the lock without the anon_vma getting > > * freed. > > + * > > + * Relies on the full mb implied by the atomic_dec_and_test() from > > + * put_anon_vma() against the full mb implied by mutex_trylock() from > > + * page_lock_anon_vma(). This orders: > > + * > > + * page_lock_anon_vma() VS put_anon_vma() > > + * mutex_trylock() atomic_dec_and_test() > > + * smp_mb() smp_mb() > > + * atomic_read() mutex_is_locked() > > Bah!, I thought all mutex_trylock() implementations used an atomic op > with return value (which implies a mb), but it looks like (at least*) > PPC doesn't and only provides a LOCK barrier. But, mutex_trylock() must imply the one-way barrier, otherwise it is buggy, no? IOW, page_lock_anon_vma() does: if (mutex_trylock(anon_vma->root->lock)) { ... atomic_read(&anon_vma->refcount); ... } If this atomic_read() can leak out of the critical section, then I think mutex_trylock() should be fixed. Or I misunderstood the problem completely... BTW, from https://lkml.org/lkml/2010/11/26/213 + * Similar to page_get_anon_vma() except it locks the anon_vma. ... - struct anon_vma *anon_vma = page_get_anon_vma(page); looks like, page_get_anon_vma() becomes unused. Oleg.