From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nick Piggin Subject: Re: [patch] fs: aio fix rcu lookup Date: Thu, 20 Jan 2011 07:45:42 +1100 Message-ID: References: <20110118190114.GA5070@quack.suse.cz> <20110118235236.GA14087@quack.suse.cz> <20110119132123.GC4246@quack.suse.cz> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Jan Kara , Andrew Morton , linux-fsdevel , linux-kernel@vger.kernel.org, "Paul E. McKenney" To: Jeff Moyer Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org On Thu, Jan 20, 2011 at 7:32 AM, Jeff Moyer wrote: > Nick Piggin writes: > >> On Thu, Jan 20, 2011 at 6:46 AM, Jeff Moyer wrot= e: >>> Jeff Moyer writes: >>> >>>> Jan Kara writes: >>>> >>>>> =A0But there's the second race I describe making it possible >>>>> for new IO to be created after io_destroy() has waited for all IO= to >>>>> finish... >>>> >>>> Can't that be solved by introducing memory barriers around the acc= esses >>>> to ->dead? >>> >>> Upon further consideration, I don't think so. >>> >>> Given the options, I think adding the synchronize rcu to the io_des= troy >>> path is the best way forward. =A0You're already waiting for a bunch= of >>> queued I/O to finish, so there is no guarantee that you're going to >>> finish that call quickly. >> >> I think synchronize_rcu() is not something to sprinkle around outsid= e >> very slow paths. It can be done without synchronize_rcu. > > I'm not sure I understand what you're saying. =A0Do you mean to imply= that > io_destroy is not a very slow path? =A0Because it is. =A0I prefer a s= olution > that doesn't re-architecht things in order to solve a theoretical iss= ue > that's never been observed. Even something that happens once per process lifetime, like in fork/exi= t is not necessarily suitable for RCU. I don't know exactly how all progr= ams use io_destroy -- of the small number that do, probably an even smaller number would care here. But I don't think it simplifies things enough t= o use synchronize_rcu for it.