On Thu, Dec 14, 2017 at 04:58:09AM -0800, Matthew Wilcox wrote: > On Wed, Dec 13, 2017 at 05:03:00PM -0800, Andrew Morton wrote: > > > sched/wait: assert the wait_queue_head lock is held in > > > __wake_up_common > > > > > > Better ensure we actually hold the lock using lockdep than just > > > commenting > > > on it. Due to the various exported _locked interfaces it is far too > > > easy > > > to get the locking wrong. > > > > I'm probably sitting on an older version. I've dropped > > > > epoll: use the waitqueue lock to protect ep->wq > > sched/wait: assert the wait_queue_head lock is held in __wake_up_common > > Looks pretty clear to me that userfaultfd is also abusing the wake_up_locked > interfaces: > > spin_lock(&ctx->fault_pending_wqh.lock); > __wake_up_locked_key(&ctx->fault_pending_wqh, TASK_NORMAL, &range); > __wake_up_locked_key(&ctx->fault_wqh, TASK_NORMAL, &range); > spin_unlock(&ctx->fault_pending_wqh.lock); > > Sure, it's locked, but not by the lock you thought it was going to be. > > There doesn't actually appear to be a bug here; fault_wqh is always serialised > by fault_pending_wqh.lock, but lockdep can't know that. I think this patch > will solve the problem.
Or userfaultfd could just always use the waitqueue lock, similar to what we are doing in epoll. But unless someone care about micro-optimizatations I'm tempted to add your patch to the next iteration of the series.
