* Grygorii Strashko <grygorii.strashko@xxxxxx> [170112 14:53]: > > > On 01/12/2017 04:19 PM, Tony Lindgren wrote: > > * Grygorii Strashko <grygorii.strashko@xxxxxx> [170112 13:54]: > >> On 01/12/2017 03:30 PM, Tony Lindgren wrote: > >> > >> Sry, but even if it looks better it might still race with PM runtime :( > >> > >>> - if (likely(pm_runtime_active(cdd->ddev.dev))) > >>> + if (likely(atomic_read(&cdd->active))) > >>> push_desc_queue(c); > >>> else > >> > >> > >> - CPU is here (-EINPROGRESS and active == 0) and then preempted > >> - PM runtime will finish cppi41_runtime_resume and clean-up pending descs > >> - CPU return here and adds desc to the pending queue > >> - oops > >> > >> Am I wrong? > > > > We had cppi41_dma_issue_pending() getting called from atomic contex and > > cppi41_runtime_resume() getting preempted where cppi41_dma_issue_pending() > > would add to the queue. > > Again, I can be mistaken but cppi41_configure_channel() seems not atomic. > cppi41_configure_channel()->dma_async_issue_pending() > + documentation says "This function can be called in an interrupt context" > > And definitely it will be preemptive on RT :( Hmm OK. So are you thinking we should add a spinlock around the test in cppi41_dma_issue_pending() and when modifying cdd->active? Something like: spin_lock_irqsave(&cdd->lock, flags); if (likely(cdd->active)) push_desc_queue(c); else list_add_tail(&c->node, &cdd->pending); spin_unlock_irqrestore(&cdd->lock, flags); Or do you have something better in mind? Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
