Re: [PATCH v4 8/8] iio: accel: sca3000: convert to guard(mutex)

[Jonathan Cameron <jic23@kernel.org>]

On Sat, 14 Mar 2026 22:36:09 +0530
Rajveer Chaudhari <rajveer.chaudhari.linux@gmail.com> wrote:

> Replace manual mutex_lock/mutex_unlock pair with guard(mutex) in
> sca3000_print_rev(), sca3000_ring_int_process(),
> sca3000_read_event_config(), __sca3000_hw_ring_state_set(),
> sca3000_hw_ring_preenable(), sca3000_hw_ring_postdisable(),
> sca3000_clean_setup(), sca3000_stop_all_interrupts().
> This ensures the mutex is released on all return paths and
> allows returning directly without a goto label.
> 
> Signed-off-by: Rajveer Chaudhari <rajveer.chaudhari.linux@gmail.com>
Hi Rajveer,

The discussion about time to review these is particularly relevant here.
Unless I'm missing something you introduce a deadlock and more radical
surgery is needed to avoid that.

Jonathan

>  
>  /**
> @@ -1313,26 +1304,20 @@ static int sca3000_hw_ring_preenable(struct iio_dev *indio_dev)
>  	int ret;
>  	struct sca3000_state *st = iio_priv(indio_dev);
>  
> -	mutex_lock(&st->lock);
> +	guard(mutex)(&st->lock);
>  
>  	/* Enable the 50% full interrupt */
>  	ret = sca3000_read_data_short(st, SCA3000_REG_INT_MASK_ADDR, 1);
>  	if (ret)
> -		goto error_unlock;
> +		return ret;
> +
>  	ret = sca3000_write_reg(st,
>  				SCA3000_REG_INT_MASK_ADDR,
>  				st->rx[0] | SCA3000_REG_INT_MASK_RING_HALF);
>  	if (ret)
> -		goto error_unlock;
> -
> -	mutex_unlock(&st->lock);
> +		return ret;
This smells interesting... The scope has expanded.

Deadlock I believe as the guard() hasn't released here before
the one in __sca3000_hw_ring_state_set) tries to grab the same
lock.

The obvious fix would be a precursor patch to update the locking
so that doesn't happen.  There is always a dance where the lock
is released before making that call (or taken after it) so just
make it __must_hold() for the lock and don't take it again internally.


>  
>  	return __sca3000_hw_ring_state_set(indio_dev, 1);
> -
> -error_unlock:
> -	mutex_unlock(&st->lock);
> -
> -	return ret;
>  }