Re: [PATCH] can: mcp251x: fix deadlock if an interrupt occurs during mcp251x_open

[Przemek Kitszel <przemyslaw.kitszel@intel.com>]

On 8/17/24 17:45, Simon Arlott wrote:
> The mcp251x_hw_wake() function is called with the mpc_lock mutex held and
> disables the interrupt handler so that no interrupts can be processed while
> waking the device. If an interrupt has already occurred then waiting for
> the interrupt handler to complete will deadlock because it will be trying
> to acquire the same mutex.
> 
> CPU0                           CPU1
> ----                           ----
> mcp251x_open()
>   mutex_lock(&priv->mcp_lock)
>    request_threaded_irq()
>                                 <interrupt>
>                                 mcp251x_can_ist()
>                                  mutex_lock(&priv->mcp_lock)
>    mcp251x_hw_wake()
>     disable_irq() <-- deadlock
> 
> Use disable_irq_nosync() instead because the interrupt handler does
> everything while holding the mutex so it doesn't matter if it's still
> running.
> 
> Signed-off-by: Simon Arlott <simon@octiron.net>

You have to provide a Fixes: tag for bugfixes [PATCH net]

otherwise the change looks fine for me,
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>

> ---
>   drivers/net/can/spi/mcp251x.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/can/spi/mcp251x.c b/drivers/net/can/spi/mcp251x.c
> index 3b8736ff0345..ec5c64006a16 100644
> --- a/drivers/net/can/spi/mcp251x.c
> +++ b/drivers/net/can/spi/mcp251x.c
> @@ -752,7 +752,7 @@ static int mcp251x_hw_wake(struct spi_device *spi)
>   	int ret;
>   
>   	/* Force wakeup interrupt to wake device, but don't execute IST */
> -	disable_irq(spi->irq);
> +	disable_irq_nosync(spi->irq);
>   	mcp251x_write_2regs(spi, CANINTE, CANINTE_WAKIE, CANINTF_WAKIF);
>   
>   	/* Wait for oscillator startup timer after wake up */