[PATCH v4] ALSA: usb-audio: qcom: Adjust mutex unlock order

[PATCH v4] ALSA: usb-audio: qcom: Adjust mutex unlock order

[Erick Karanja]

The mutexes qdev_mutex and chip->mutex are acquired in that order
throughout the driver. To preserve proper lock hierarchy and avoid
potential deadlocks, they must be released in the reverse
order of acquisition.

This change reorders the unlock sequence to first release chip->mutex
followed by qdev_mutex, ensuring consistency with the locking pattern.

Fixes: 326bbc348298a ("ALSA: usb-audio: qcom: Introduce QC USB
                      SND offloading support")

changes since v3:
 - Add the correct fixes tag.

Signed-off-by: Erick Karanja <karanja99erick@gmail.com>
---
 sound/usb/qcom/qc_audio_offload.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sound/usb/qcom/qc_audio_offload.c b/sound/usb/qcom/qc_audio_offload.c
index 3543b5a53592..711f5612a83a 100644
--- a/sound/usb/qcom/qc_audio_offload.c
+++ b/sound/usb/qcom/qc_audio_offload.c
@@ -825,8 +825,8 @@ static int uaudio_sideband_notifier(struct usb_interface *intf,
 		}
 	}
 
-	mutex_unlock(&qdev_mutex);
 	mutex_unlock(&chip->mutex);
+	mutex_unlock(&qdev_mutex);
 
 	return 0;
 }
@@ -1865,8 +1865,8 @@ static void qc_usb_audio_offload_disconnect(struct snd_usb_audio *chip)
 
 	/* Device has already been cleaned up, or never populated */
 	if (!dev->chip) {
-		mutex_unlock(&qdev_mutex);
 		mutex_unlock(&chip->mutex);
+		mutex_unlock(&qdev_mutex);
 		return;
 	}
 
@@ -1921,8 +1921,8 @@ static void qc_usb_audio_offload_suspend(struct usb_interface *intf,
 
 	uaudio_send_disconnect_ind(chip);
 
+        mutex_unlock(&chip->mutex);
 	mutex_unlock(&qdev_mutex);
-	mutex_unlock(&chip->mutex);
 }
 
 static struct snd_usb_platform_ops offload_ops = {
-- 
2.43.0

Re: [PATCH v4] ALSA: usb-audio: qcom: Adjust mutex unlock order

[Markus Elfring]

…
> This change reorders the unlock sequence to first release chip->mutex
> followed by qdev_mutex, ensuring consistency with the locking pattern.
> 
> Fixes: 326bbc348298a ("ALSA: usb-audio: qcom: Introduce QC USB
>                       SND offloading support")
> 
> changes since v3:
>  - Add the correct fixes tag.
…

Please take another look at patch requirements.

* https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.16-rc6#n94

* https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.16-rc6#n145

* https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.16-rc6#n784


Regards,
Markus

Re: [PATCH v4] ALSA: usb-audio: qcom: Adjust mutex unlock order

[Markus Elfring]

> The mutexes qdev_mutex and chip->mutex are acquired in that order
> throughout the driver. To preserve proper lock hierarchy and avoid
> potential deadlocks, they must be released in the reverse
> order of acquisition.

Would you become interested to benefit any more from lock guards?
https://elixir.bootlin.com/linux/v6.16-rc6/source/include/linux/mutex.h#L225

Regards,
Markus

Re: [PATCH v4] ALSA: usb-audio: qcom: Adjust mutex unlock order

[Takashi Iwai]

On Mon, 21 Jul 2025 13:45:53 +0200,
Erick Karanja wrote:
> 
> The mutexes qdev_mutex and chip->mutex are acquired in that order
> throughout the driver. To preserve proper lock hierarchy and avoid
> potential deadlocks, they must be released in the reverse
> order of acquisition.
> 
> This change reorders the unlock sequence to first release chip->mutex
> followed by qdev_mutex, ensuring consistency with the locking pattern.
> 
> Fixes: 326bbc348298a ("ALSA: usb-audio: qcom: Introduce QC USB
>                       SND offloading support")

This has to be a single line without folding.

> changes since v3:
>  - Add the correct fixes tag.
> 
> Signed-off-by: Erick Karanja <karanja99erick@gmail.com>
> ---
>  sound/usb/qcom/qc_audio_offload.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/sound/usb/qcom/qc_audio_offload.c b/sound/usb/qcom/qc_audio_offload.c
> index 3543b5a53592..711f5612a83a 100644
> --- a/sound/usb/qcom/qc_audio_offload.c
> +++ b/sound/usb/qcom/qc_audio_offload.c
> @@ -825,8 +825,8 @@ static int uaudio_sideband_notifier(struct usb_interface *intf,
>  		}
>  	}
>  
> -	mutex_unlock(&qdev_mutex);
>  	mutex_unlock(&chip->mutex);
> +	mutex_unlock(&qdev_mutex);
>  
>  	return 0;
>  }
> @@ -1865,8 +1865,8 @@ static void qc_usb_audio_offload_disconnect(struct snd_usb_audio *chip)
>  
>  	/* Device has already been cleaned up, or never populated */
>  	if (!dev->chip) {
> -		mutex_unlock(&qdev_mutex);
>  		mutex_unlock(&chip->mutex);
> +		mutex_unlock(&qdev_mutex);
>  		return;
>  	}
>  
> @@ -1921,8 +1921,8 @@ static void qc_usb_audio_offload_suspend(struct usb_interface *intf,
>  
>  	uaudio_send_disconnect_ind(chip);
>  
> +        mutex_unlock(&chip->mutex);
>  	mutex_unlock(&qdev_mutex);
> -	mutex_unlock(&chip->mutex);

The above used spaces instead of tabs, and I corrected in my side.
Such a failure could be spotted out by running scripts/checkpatch.pl.
At the next time, please run and test it before submissions.

In anyway, I applied the patch now.


thanks,

Takashi

Re: [PATCH v4] ALSA: usb-audio: qcom: Adjust mutex unlock order

[Markus Elfring]

…
> > +++ b/sound/usb/qcom/qc_audio_offload.c
…
> > @@ -1865,8 +1865,8 @@ static void qc_usb_audio_offload_disconnect(struct snd_usb_audio *chip)
> >  
> >  	/* Device has already been cleaned up, or never populated */
> >  	if (!dev->chip) {
> > -		mutex_unlock(&qdev_mutex);
> >  		mutex_unlock(&chip->mutex);
> > +		mutex_unlock(&qdev_mutex);
> >  		return;
> >  	}
> >  
…
> In anyway, I applied the patch now.

How do you think about to refine this function implementation another bit
by using an additional label like “unlock” rather than keeping duplicate
source code?


Regards,
Markus