Re: [PATCH RFC] fs/aio: fix sleeping while TASK_INTERRUPTIBLE

[Kent Overstreet <kmo@daterainc.com>]

On Wed, Dec 24, 2014 at 10:11:34PM -0500, 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.

Ah - and then you set the task state back to TASK_RUNNING if _any_ events were
found... yeah, I guess that seems safe. Probably worth a few comments, though :)

--
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: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>