From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Mason Subject: Re: [PATCH RFC] fs/aio: fix sleeping while TASK_INTERRUPTIBLE Date: Sun, 28 Dec 2014 20:24:26 -0500 Message-ID: <1419816266.13012.15@mail.thefacebook.com> References: <20141223001619.GA26385@ret.masoncoding.com> <20141223184328.GB17185@kvack.org> <1419360926.13012.12@mail.thefacebook.com> <20141223215847.GD17185@kvack.org> <20141225025958.GA30938@moria.home.lan> <20141225031134.GI17185@kvack.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; format=flowed Cc: Kent Overstreet , , , Peter Zijlstra To: Benjamin LaHaise Return-path: In-Reply-To: <20141225031134.GI17185@kvack.org> Sender: owner-linux-aio@kvack.org List-Id: linux-fsdevel.vger.kernel.org On Wed, Dec 24, 2014 at 10:11 PM, Benjamin LaHaise wrote: > On Wed, Dec 24, 2014 at 06:59:58PM -0800, Kent Overstreet wrote: >> On Tue, Dec 23, 2014 at 04:58:47PM -0500, Benjamin LaHaise wrote: >> > Hi Chris, >> > >> > On Tue, Dec 23, 2014 at 01:55:26PM -0500, Chris Mason wrote: >> > > Works for me, the patch is mostly a (somewhat commented) list >> of all >> > > the places we're currently doing it wrong. >> > >> > I think the following change should suffice to fix this issue, >> and it's a >> > lot easier to review, too. I've given this a quick test, and it >> works for >> > me. I do have one concern: is it safe to call mutex_lock() when >> the current >> > task is already on other wait queues? If the answer is no, then >> it may be >> > necessary to convert ->ring_lock back into spinlock as it was >> prior to 3.10 >> > to avoid using mutex_lock(). The same question applies to kmap() >> and >> > copy_to_user(), and those concerns might have implications across >> the rest >> > of the kernel. Thoughts? >> > >> > + __set_current_state(state); >> >> I don't think this is safe - if we race, and another thread wakes >> us up, we're >> setting our state back to TASK_INTERRUPTIBLE _without_ us being on >> the waitlist. > > It should be -- the conditions for going to sleep are checked after > current's > state is set here. We're task interruptible and may be run under a timer, so we'll catch wakeups for signals and timers in addition to available aio events. -chris -- To unsubscribe, send a message with 'unsubscribe linux-aio' in the body to majordomo@kvack.org. For more info on Linux AIO, see: http://www.kvack.org/aio/ Don't email: aart@kvack.org