[prev in list] [next in list] [prev in thread] [next in thread] 

List:       linux-omap
Subject:    Re: musb RPM sleep-while-atomic in 4.9-rc1
From:       Johan Hovold <johan () kernel ! org>
Date:       2016-10-31 11:49:21
Message-ID: 20161031114921.GB4254 () localhost
[Download RAW message or body]

On Fri, Oct 28, 2016 at 11:13:19AM -0700, Tony Lindgren wrote:
> * Johan Hovold <johan@kernel.org> [161028 02:45]:
> > On Thu, Oct 27, 2016 at 12:15:52PM -0700, Tony Lindgren wrote:
> > > * Johan Hovold <johan@kernel.org> [161027 11:46]:
> > > > But then this looks like it could trigger an ABBA deadlock as musb->lock
> > > > is held while queue_on_resume() takes musb->list_lock, and
> > > > musb_run_pending() would take the same locks in the reverse order.
> > > 
> > > It seems we can avoid that by locking only list_add_tail() and list_del():
> > > 
> > > list_for_each_entry_safe(w, _w, &musb->resume_work, node) {
> > > 	spin_lock_irqsave(&musb->list_lock, flags);
> > > 	list_del(&w->node);
> > > 	spin_unlock_irqrestore(&musb->list_lock, flags);
> > > 	if (w->callback)
> > > 		w->callback(musb, w->data);
> > > 	devm_kfree(musb->controller, w);
> > > }
> > 
> > I think you still need to hold the lock while traversing the list (even
> > if you temporarily release it during the callback).
> 
> Hmm yeah we need iterate through the list again to avoid missing new
> elements being added. I've updated the patch to use a the common
> while (!list_empty(&musb->resume_work)) loop. Does that solve the
> concern you had or did you also had some other concern there?

Yeah, while that minimises the race window it is still possible that the
timer callback checks pm_runtime_active() after the queue has been
processed but before the rpm status is updated. 

How about using a work struct and synchronous get for the deferred case?

Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
[prev in list] [next in list] [prev in thread] [next in thread]