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.

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

diff --git a/sound/usb/qcom/qc_audio_offload.c b/sound/usb/qcom/qc_audio_offload.c
index 3543b5a53592..ef144d2be7d2 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;
 }
-- 
2.43.0

On Wed 2025-07-16 09:23:30, 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.

Please explain how the deadlock would happen.

								Pavel
-- 
I don't work for Nazis and criminals, and neither should you.
Boycott Putin, Trump, and Musk!

On Wed, 16 Jul 2025 08:23:30 +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.
> 
> Signed-off-by: Erick Karanja <karanja99erick@gmail.com>
> ---
>  sound/usb/qcom/qc_audio_offload.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/sound/usb/qcom/qc_audio_offload.c b/sound/usb/qcom/qc_audio_offload.c
> index 3543b5a53592..ef144d2be7d2 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;
>  }

The same pattern is found in qc_usb_audio_offload_disconnect() and
qc_usb_audio_offload_suspend(), too.
Care to address there as well?

Maybe it's better to replace with guard stuff, but it should be done
by another patch in later kernel releases.


thanks,

Takashi

On Wed, 2025-07-16 at 08:35 +0200, Takashi Iwai wrote:
> On Wed, 16 Jul 2025 08:23:30 +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.
> > 
> > Signed-off-by: Erick Karanja <karanja99erick@gmail.com>
> > ---
> >  sound/usb/qcom/qc_audio_offload.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/sound/usb/qcom/qc_audio_offload.c
> > b/sound/usb/qcom/qc_audio_offload.c
> > index 3543b5a53592..ef144d2be7d2 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;
> >  }
> 
> The same pattern is found in qc_usb_audio_offload_disconnect() and
> qc_usb_audio_offload_suspend(), too.
> Care to address there as well?
Yes, working on it.
> 
> Maybe it's better to replace with guard stuff, but it should be done
> by another patch in later kernel releases.
> 
> 
> thanks,
> 
> Takashi