Re: [PATCH] usb: cdnsp: Fix deadlock issue in cdnsp_thread_irq_handler

[Peter Chen <peter.chen@kernel.org>]

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?

Peter
> 
> 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);
>  		return IRQ_HANDLED;
>  	}
>  
> @@ -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