Re: [PATCH v3] mtd: spi-nor: use guard() in spi_nor_rww_start_exclusive

[Tudor Ambarus]

On 8/11/26 11:31 AM, Runyu Xiao wrote:
> Commit 03e7bb864d9a ("mtd: spi-nor: use scope-based mutex cleanup
> helpers") converted the RWW start helpers to scoped mutex cleanup, but
> left spi_nor_rww_start_exclusive() using a plain mutex_lock().
> 
> spi_nor_rww_start_exclusive() is used as a wait_event_killable()
> condition. When it returns false on the busy path, the raw mutex_lock()
> leaves nor->lock held and blocks the operation that must clear the RWW
> state in spi_nor_rww_end_exclusive().
> 
> Use guard(mutex) here as well so this helper matches the other RWW
> start helpers and releases nor->lock on the false return path.
> 
> Fixes: 03e7bb864d9a ("mtd: spi-nor: use scope-based mutex cleanup helpers")

The fixes tag is wrong. The blamed commit did not change the functionality
of the code, while yours does.

You shall instead blame the commit that failed to release the lock in the
first place. But then if you want to have this backported to stable kernels
you may want to add 03e7bb864d9a ("mtd: spi-nor: use scope-based mutex
cleanup helpers") as a prerequisite patch, as it includes <linux/cleanup.h>.

Cheers,
ta

> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Runyu Xiao <runyu.xiao@xxxxxxxxxx>
> ---
> Changes in v3:
> - Reframe the changelog around the scoped-helper conversion that missed
>   spi_nor_rww_start_exclusive().
> 
> Changes in v2:
> - Replace the analysis-style explanation with a short maintainer-facing
>   description of the busy-path lock hold.
> - Keep the fix focused on using guard(mutex) so the mutex is released on
>   both paths.
> 
> drivers/mtd/spi-nor/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
> index ccf4396cdcd0..8bc117b46e02 100644
> --- a/drivers/mtd/spi-nor/core.c
> +++ b/drivers/mtd/spi-nor/core.c
> @@ -1310,7 +1310,7 @@ static bool spi_nor_rww_start_exclusive(struct spi_nor *nor)
>  {
>  	struct spi_nor_rww *rww = &nor->rww;
>  
> -	mutex_lock(&nor->lock);
> +	guard(mutex)(&nor->lock);
>  
>  	if (rww->ongoing_io || rww->ongoing_rd || rww->ongoing_pe)
>  		return false;