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>

1 files changed, 4 insertions, 4 deletions

diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c
index 3952f2853703..102ac4a94a9d 100644
--- a/sound/pci/hda/cs35l41_hda.c
+++ b/sound/pci/hda/cs35l41_hda.c
@@ -91,20 +91,18 @@ static const struct reg_sequence cs35l41_hda_mute[] = {
 	{ CS35L41_AMP_DIG_VOL_CTRL,	0x0000A678 }, // AMP_VOL_PCM Mute
 };
 
-static int cs35l41_control_add(struct cs_dsp_coeff_ctl *cs_ctl)
+static void cs35l41_add_controls(struct cs35l41_hda *cs35l41)
 {
-	struct cs35l41_hda *cs35l41 = container_of(cs_ctl->dsp, struct cs35l41_hda, cs_dsp);
 	struct hda_cs_dsp_ctl_info info;
 
 	info.device_name = cs35l41->amp_name;
 	info.fw_type = cs35l41->firmware_type;
 	info.card = cs35l41->codec->card;
 
-	return hda_cs_dsp_control_add(cs_ctl, &info);
+	hda_cs_dsp_add_controls(&cs35l41->cs_dsp, &info);
 }
 
 static const struct cs_dsp_client_ops client_ops = {
-	.control_add = cs35l41_control_add,
 	.control_remove = hda_cs_dsp_control_remove,
 };
 
@@ -435,6 +433,8 @@ static int cs35l41_init_dsp(struct cs35l41_hda *cs35l41)
 	if (ret)
 		goto err_release;
 
+	cs35l41_add_controls(cs35l41);
+
 	ret = cs35l41_save_calibration(cs35l41);
 
 err_release: