On 21-05-20 11:42:24, Pawel Laszczak wrote:
From: Pawel Laszczak pawell@cadence.com
Patch fixes the critical issue caused by deadlock which has been detected during testing NCM class.
The root cause of issue is spin_lock/spin_unlock instruction instead spin_lock_irqsave/spin_lock_irqrestore in cdnsp_thread_irq_handler function.
Pawel, would you please explain more about why the deadlock occurs with current code, and why this patch could fix it?
I'm going to add such description to commit message:
Lack of spin_lock_irqsave causes that during handling threaded interrupt inside spin_lock/spin_unlock section the ethernet subsystem invokes eth_start_xmit function from interrupt context which in turn starts queueing free requests in cdnsp driver. Consequently, the deadlock occurs inside cdnsp_gadget_ep_queue on spin_lock_irqsave instruction. Replacing spin_lock/spin_unlock with spin_lock_irqsave/spin_lock_irqrestor causes disableing interrupts and blocks queuing requests by ethernet subsystem until cdnsp_thread_irq_handler ends..
I hope this description is sufficient.
Thanks, Pawel
Cc: stable@vger.kernel.org Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP DRD Driver") Signed-off-by: Pawel Laszczak pawell@cadence.com
drivers/usb/cdns3/cdnsp-ring.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/cdns3/cdnsp-ring.c b/drivers/usb/cdns3/cdnsp-ring.c index 5f0513c96c04..68972746e363 100644 --- a/drivers/usb/cdns3/cdnsp-ring.c +++ b/drivers/usb/cdns3/cdnsp-ring.c @@ -1517,13 +1517,14 @@ irqreturn_t cdnsp_thread_irq_handler(int irq, void *data) { struct cdnsp_device *pdev = (struct cdnsp_device *)data; union cdnsp_trb *event_ring_deq;
- unsigned long flags; int counter = 0;
- spin_lock(&pdev->lock);
spin_lock_irqsave(&pdev->lock, flags);
if (pdev->cdnsp_state & (CDNSP_STATE_HALTED | CDNSP_STATE_DYING)) { cdnsp_died(pdev);
spin_unlock(&pdev->lock);
spin_unlock_irqrestore(&pdev->lock, flags);@@ -1539,7 +1540,7 @@ irqreturn_t cdnsp_thread_irq_handler(int irq, void *data)
cdnsp_update_erst_dequeue(pdev, event_ring_deq, 1);
- spin_unlock(&pdev->lock);
spin_unlock_irqrestore(&pdev->lock, flags);
return IRQ_HANDLED;
}
2.25.1
--
Thanks, Peter Chen