Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759462AbXELRF0 (ORCPT ); Sat, 12 May 2007 13:05:26 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1756641AbXELRFR (ORCPT ); Sat, 12 May 2007 13:05:17 -0400 Received: from viefep13-int.chello.at ([213.46.255.15]:26981 "EHLO viefep17-int.chello.at" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1756423AbXELRFQ (ORCPT ); Sat, 12 May 2007 13:05:16 -0400 Subject: Re: [PATCH 1/2] scalable rw_mutex From: Peter Zijlstra To: Oleg Nesterov Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Andrew Morton , Ingo Molnar , Thomas Gleixner , Nick Piggin In-Reply-To: <20070512160428.GA173@tv-sign.ru> References: <20070511131541.992688403@chello.nl> <20070511132321.895740140@chello.nl> <20070511230023.GA449@tv-sign.ru> <1178977276.6810.59.camel@twins> <20070512160428.GA173@tv-sign.ru> Content-Type: text/plain Date: Sat, 12 May 2007 18:57:47 +0200 Message-Id: <1178989068.19461.3.camel@lappy> Mime-Version: 1.0 X-Mailer: Evolution 2.8.1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2403 Lines: 85 On Sat, 2007-05-12 at 20:04 +0400, Oleg Nesterov wrote: > On 05/12, Peter Zijlstra wrote: > > > > +static inline void rw_mutex_readers_dec(struct rw_mutex *rw_mutex) > > +{ > > + percpu_counter_dec(&rw_mutex->readers); > > + smp_wmb(); > > +} > > > > +void rw_mutex_read_unlock(struct rw_mutex *rw_mutex) > > +{ > > + rw_mutex_readers_dec(rw_mutex); > > + /* > > + * on the slow path; > > + * nudge the writer waiting for the last reader to go away > > + */ > > + if (unlikely(rw_mutex_reader_slow(rw_mutex))) > > + rw_mutex_writer_wake(rw_mutex); > > +} > > > > +void rw_mutex_write_lock_nested(struct rw_mutex *rw_mutex, int subclass) > > +{ > > + mutex_lock_nested(&rw_mutex->write_mutex, subclass); > > + > > + /* > > + * block new readers > > + */ > > + mutex_lock_nested(&rw_mutex->read_mutex, subclass); > > + rw_mutex_status_set(rw_mutex, RW_MUTEX_READER_SLOW); > > + /* > > + * and wait for all current readers to go away > > + */ > > + rw_mutex_writer_wait(rw_mutex, (rw_mutex_readers(rw_mutex) == 0)); > > +} > > I think this is still not right, but when it comes to barriers we > need a true expert (Paul cc-ed). > > this code roughly does (the only reader does unlock) > > READER WRITER > > readers = 0; state = 1; > wmb(); wmb(); > CHECK(state != 0) CHECK(readers == 0) > > We need to ensure that we can't miss both CHECKs. Either reader > should see RW_MUTEX_READER_SLOW, o writer sees "readers == 0" > and does not sleep. > > In that case both barriers should be converted to smp_mb(). There > was a _long_ discussion about STORE-MB-LOAD behaviour, and experts > seem to believe everething is ok. Ah, but note that both those CHECK()s have a rmb(), so that ends up being: READER WRITER readers = 0; state = 1; wmb(); wmb(); rmb(); rmb(); if (state != 0) if (readers == 0) and a wmb+rmb is a full mb, right? > Another question. Isn't it possible to kill rw_mutex->status ? > > I have a vague feeling you can change the code so that > > rw_mutex_reader_slow() <=> "->waiter != NULL" > > , but I am not sure. If not now, it might be possible to make it so. Thanks for the suggestion. Peter - 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/