From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Paul E. McKenney" Subject: Re: [PATCH 4/6] kvm tools: Add rwlock wrapper Date: Sun, 29 May 2011 08:51:02 -0700 Message-ID: <20110529155102.GA6893@linux.vnet.ibm.com> References: <1306491547.3217.9.camel@lappy> <20110527103657.GA25748@elte.hu> <1306511560.3217.23.camel@lappy> <20110527171040.GC4356@elte.hu> <1306527578.3217.26.camel@lappy> <20110528152408.GA27104@elte.hu> <1306611908.3282.7.camel@lappy> <4DE1EC05.3040001@redhat.com> <20110529071948.GA20686@elte.hu> <20110529153130.GG2668@linux.vnet.ibm.com> Reply-To: paulmck@linux.vnet.ibm.com Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Avi Kivity , Sasha Levin , Mathieu Desnoyers , Pekka Enberg , john@jfloren.net, kvm@vger.kernel.org, asias.hejun@gmail.com, gorcunov@gmail.com, prasadjoshi124@gmail.com To: Ingo Molnar Return-path: Received: from e8.ny.us.ibm.com ([32.97.182.138]:58631 "EHLO e8.ny.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753754Ab1E2PvH (ORCPT ); Sun, 29 May 2011 11:51:07 -0400 Received: from d01relay02.pok.ibm.com (d01relay02.pok.ibm.com [9.56.227.234]) by e8.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id p4TFeJIl015211 for ; Sun, 29 May 2011 11:40:19 -0400 Received: from d01av01.pok.ibm.com (d01av01.pok.ibm.com [9.56.224.215]) by d01relay02.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p4TFp6hM482734 for ; Sun, 29 May 2011 11:51:06 -0400 Received: from d01av01.pok.ibm.com (loopback [127.0.0.1]) by d01av01.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p4TFp4RA003403 for ; Sun, 29 May 2011 11:51:06 -0400 Content-Disposition: inline In-Reply-To: <20110529153130.GG2668@linux.vnet.ibm.com> Sender: kvm-owner@vger.kernel.org List-ID: On Sun, May 29, 2011 at 08:31:30AM -0700, Paul E. McKenney wrote: > On Sun, May 29, 2011 at 09:19:48AM +0200, Ingo Molnar wrote: > > > > * Avi Kivity wrote: > > > > > Yes, this is equivalent to the kernel's stop_machine_run(). It's a > > > heavyweight method but it should work just fine. > > > > Yeah. It is fine for reconfiguration/configuration-only kind of write > > paths - i.e. the mmio lookup path should be ok. > > > > There's only one thing i'd like to warn about: please still shape it > > as a br_read_lock()/unlock(), br_write_lock()/unlock() operation. > > > > This way all the SMP read paths remain identified properly, even if > > br_write_lock() does a stop_machine_run() equivalent. This still > > leaves options open for an easy conversion to rwlock or urcu (should > > any particular write path become more than just occasional). > > This is very important even if no write path ever becomes more than > just occasional. If you don't mark the read paths like Ingo suggests, > your one-year-from-now self will be very annoyed at you, as the code > will be quite difficult to understand. And anyone else trying to > read your code will be even more annoyed. ;-) And the other reason that you want to mark the readers is for debug purposes. Murphy being who he is, you will some day need to check for someone calling the "OK to update" function while they are acting as a reader. Thanx, Paul