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 02:10:37PM +0100, Christoph Hellwig wrote:
> On Thu, Dec 14, 2017 at 04:58:09AM -0800, Matthew Wilcox wrote:
> > 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.

userfaultfd is using the waitqueue lock -- it just has two waitqueues
that it's protecting with the same lock.

If the patch goes through as-is, try this changelog:

[PATCH] userfaultfd: Use fault_wqh lock

userfaultfd was using the fault_pending_wq lock to protect both
fault_pending_wq and fault_wqh. With Christoph's addition of a lockdep
assert to the wait queue code, that will trigger warnings (although there
is no bug). Remove the warning by using __wake_up which will take the
fault_wqh lock. This lock now nests inside the fault_pending_wqh lock,
but that's not a problem since it was entireyl unused before.

Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>

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