Searched hist:"2176 c6b599dba55a640cffec0182c0b6bab680d1" (Results 1 – 3 of 3) sorted by relevance
/openbmc/linux/sound/pci/hda/ |
H A D | hda_cs_dsp_ctl.h | diff 2176c6b599dba55a640cffec0182c0b6bab680d1 Tue Oct 11 09:35:50 CDT 2022 Richard Fitzgerald <rf@opensource.cirrus.com> ALSA: hda/cs_dsp_ctl: Fix mutex inversion when creating controls
Redesign the creation of ALSA controls so that the cs_dsp pwr_lock is not held when calling snd_ctl_add(). Instead of creating the ALSA control from the cs_dsp control_add callback, do it after cs_dsp_power_up() has completed. The existing functions are changed to return void instead of passing errors back - this duplicates the original behaviour, as cs_dsp does not abort firmware load if creation of a control fails.
It is safe to walk the control list without taking any mutex provided that the caller is not trying to load a new firmware or remove the driver in parallel. There is no other situation that the list can change. So the caller can trigger creation of ALSA controls after cs_dsp_power_up() has returned. A cs_dsp control will have a non-NULL priv pointer if we have created an ALSA control.
With the previous code the ALSA controls were created from the cs_dsp control_add callback. But this is called with pwr_lock held (as it is part of the DSP power-up sequence). The kernel lock checking will show a mutex inversion between this and the control creation path:
control_add pwr_lock held, takes controls_rwsem (in snd_ctl_add)
get/put controls_rwsem held, takes pwr_lock to call cs_dsp.
This is not completely theoretical. Although the time window is very small, it is possible for these to run in parallel and deadlock the old implementation.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com> Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com> Link: https://lore.kernel.org/r/20221011143552.621792-4-sbinding@opensource.cirrus.com Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
H A D | hda_cs_dsp_ctl.c | diff 2176c6b599dba55a640cffec0182c0b6bab680d1 Tue Oct 11 09:35:50 CDT 2022 Richard Fitzgerald <rf@opensource.cirrus.com> ALSA: hda/cs_dsp_ctl: Fix mutex inversion when creating controls
Redesign the creation of ALSA controls so that the cs_dsp pwr_lock is not held when calling snd_ctl_add(). Instead of creating the ALSA control from the cs_dsp control_add callback, do it after cs_dsp_power_up() has completed. The existing functions are changed to return void instead of passing errors back - this duplicates the original behaviour, as cs_dsp does not abort firmware load if creation of a control fails.
It is safe to walk the control list without taking any mutex provided that the caller is not trying to load a new firmware or remove the driver in parallel. There is no other situation that the list can change. So the caller can trigger creation of ALSA controls after cs_dsp_power_up() has returned. A cs_dsp control will have a non-NULL priv pointer if we have created an ALSA control.
With the previous code the ALSA controls were created from the cs_dsp control_add callback. But this is called with pwr_lock held (as it is part of the DSP power-up sequence). The kernel lock checking will show a mutex inversion between this and the control creation path:
control_add pwr_lock held, takes controls_rwsem (in snd_ctl_add)
get/put controls_rwsem held, takes pwr_lock to call cs_dsp.
This is not completely theoretical. Although the time window is very small, it is possible for these to run in parallel and deadlock the old implementation.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com> Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com> Link: https://lore.kernel.org/r/20221011143552.621792-4-sbinding@opensource.cirrus.com Signed-off-by: Takashi Iwai <tiwai@suse.de>
|
H A D | cs35l41_hda.c | diff 2176c6b599dba55a640cffec0182c0b6bab680d1 Tue Oct 11 09:35:50 CDT 2022 Richard Fitzgerald <rf@opensource.cirrus.com> ALSA: hda/cs_dsp_ctl: Fix mutex inversion when creating controls
Redesign the creation of ALSA controls so that the cs_dsp pwr_lock is not held when calling snd_ctl_add(). Instead of creating the ALSA control from the cs_dsp control_add callback, do it after cs_dsp_power_up() has completed. The existing functions are changed to return void instead of passing errors back - this duplicates the original behaviour, as cs_dsp does not abort firmware load if creation of a control fails.
It is safe to walk the control list without taking any mutex provided that the caller is not trying to load a new firmware or remove the driver in parallel. There is no other situation that the list can change. So the caller can trigger creation of ALSA controls after cs_dsp_power_up() has returned. A cs_dsp control will have a non-NULL priv pointer if we have created an ALSA control.
With the previous code the ALSA controls were created from the cs_dsp control_add callback. But this is called with pwr_lock held (as it is part of the DSP power-up sequence). The kernel lock checking will show a mutex inversion between this and the control creation path:
control_add pwr_lock held, takes controls_rwsem (in snd_ctl_add)
get/put controls_rwsem held, takes pwr_lock to call cs_dsp.
This is not completely theoretical. Although the time window is very small, it is possible for these to run in parallel and deadlock the old implementation.
Signed-off-by: Richard Fitzgerald <rf@opensource.cirrus.com> Signed-off-by: Stefan Binding <sbinding@opensource.cirrus.com> Link: https://lore.kernel.org/r/20221011143552.621792-4-sbinding@opensource.cirrus.com Signed-off-by: Takashi Iwai <tiwai@suse.de>
|