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 from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
