From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kara Subject: Re: [patch] fs: aio fix rcu lookup Date: Wed, 19 Jan 2011 17:50:11 +0100 Message-ID: <20110119165011.GA16682@quack.suse.cz> 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 , Jeff Moyer , Andrew Morton , linux-fsdevel , linux-kernel@vger.kernel.org To: Nick Piggin Return-path: Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org On Thu 20-01-11 03:03:23, Nick Piggin wrote: > On Thu, Jan 20, 2011 at 12:21 AM, Jan Kara wrote: > > =A0Well, we are not required to cancel all the outstanding AIO beca= use of the > > API requirement, that's granted. But we must do it because of the w= ay how > > the code is written. Outstanding IO requests reference ioctx but th= ey are > > not counted in ctx->users but in ctx->reqs_active. So the code reli= es on > > the fact that the reference held by the hash table protects ctx fro= m being > > freed and io_destroy() waits for requests before dropping the last > > reference to ctx. But there's the second race I describe making it = possible > > for new IO to be created after io_destroy() has waited for all IO t= o > > finish... >=20 > Yes there is that race too I agree. I just didn't follow through the = code far > enough to see it was a problem -- I thought it was by design. >=20 > I'd like to solve it without synchronize_rcu() though. Ah, OK. I don't find io_destroy() performance critical but I can understand that you need not like synchronize_rcu() there. ;) Then it should be possible to make IO requests count in ctx->users which would solve the race as well. We'd just have to be prepared that request completion might put the last reference to ioctx and free it but that shouldn't be an issue. Do you like that solution better? Honza --=20 Jan Kara SUSE Labs, CR