lkml.org 
[lkml]   [2017]   [Dec]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: d1fc031747 ("sched/wait: assert the wait_queue_head lock is .."): EIP: __wake_up_common
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.

\
 
 \ /
  Last update: 2017-12-14 14:11    [W:0.387 / U:0.300 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site