Re: musb RPM sleep-while-atomic in 4.9-rc1

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Oct 28, 2016 at 11:13:19AM -0700, Tony Lindgren wrote:
> * Johan Hovold <johan@xxxxxxxxxx> [161028 02:45]:
> > On Thu, Oct 27, 2016 at 12:15:52PM -0700, Tony Lindgren wrote:
> > > * Johan Hovold <johan@xxxxxxxxxx> [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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux