[PATCH] ASoC: SOF: ipc4/intel: Add missing mutex_unlock()
There was a missing mutex_unlock() in sof_ipc4_widget_free() use_chain_dma if-branch that caused a static analysis error. The branch should not be used in a normal working configuration and if its used its an indication of a bad topology. Add missing mutex_unlock() and a warning print if the if-branch is taken, and another warning print to a symmetric place in sof_ipc4_widget_setup().
Fixes: ca5ce0caa67fa9 ("ASoC: SOF: ipc4/intel: Add support for chained DMA") Reported-by: kernel test robot lkp@intel.com Reported-by: Dan Carpenter error27@gmail.com Link: https://lore.kernel.org/r/202303222050.dCw0fPCW-lkp@intel.com/ Signed-off-by: Jyri Sarha jyri.sarha@linux.intel.com --- sound/soc/sof/ipc4-topology.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/sound/soc/sof/ipc4-topology.c b/sound/soc/sof/ipc4-topology.c index f1e1aed94da4..12775fcb6b54 100644 --- a/sound/soc/sof/ipc4-topology.c +++ b/sound/soc/sof/ipc4-topology.c @@ -1989,8 +1989,11 @@ static int sof_ipc4_widget_setup(struct snd_sof_dev *sdev, struct snd_sof_widget case snd_soc_dapm_scheduler: pipeline = swidget->private;
- if (pipeline->use_chain_dma) + if (pipeline->use_chain_dma) { + dev_warn(sdev->dev, "use_chain_dma set for schduler %s", + swidget->widget->name); return 0; + }
dev_dbg(sdev->dev, "pipeline: %d memory pages: %d\n", swidget->pipeline_id, pipeline->mem_usage); @@ -2145,8 +2148,12 @@ static int sof_ipc4_widget_free(struct snd_sof_dev *sdev, struct snd_sof_widget struct sof_ipc4_msg msg = {{ 0 }}; u32 header;
- if (pipeline->use_chain_dma) + if (pipeline->use_chain_dma) { + dev_warn(sdev->dev, "use_chain_dma set for schduler %s", + swidget->widget->name); + mutex_unlock(&ipc4_data->pipeline_state_mutex); return 0; + }
header = SOF_IPC4_GLB_PIPE_INSTANCE_ID(swidget->instance_id); header |= SOF_IPC4_MSG_TYPE_SET(SOF_IPC4_GLB_DELETE_PIPELINE);
On Wed, 22 Mar 2023 20:18:30 +0200, Jyri Sarha wrote:
There was a missing mutex_unlock() in sof_ipc4_widget_free() use_chain_dma if-branch that caused a static analysis error. The branch should not be used in a normal working configuration and if its used its an indication of a bad topology. Add missing mutex_unlock() and a warning print if the if-branch is taken, and another warning print to a symmetric place in sof_ipc4_widget_setup().
[...]
Applied to
broonie/sound.git for-next
Thanks!
[1/1] ASoC: SOF: ipc4/intel: Add missing mutex_unlock() commit: 935d31fdda2c69324b3eeb648a73fdedf4131474
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From: Markus Elfring elfring@users.sourceforge.net Date: Sat, 10 Jun 2023 12:40:09 +0200
Add a jump target so that a call of the function “mutex_unlock” is stored only once in this function implementation.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net --- sound/soc/sof/ipc4-topology.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/sound/soc/sof/ipc4-topology.c b/sound/soc/sof/ipc4-topology.c index a4e1a70b607d..f0fd1dfa384e 100644 --- a/sound/soc/sof/ipc4-topology.c +++ b/sound/soc/sof/ipc4-topology.c @@ -2300,8 +2300,7 @@ static int sof_ipc4_widget_free(struct snd_sof_dev *sdev, struct snd_sof_widget if (pipeline->use_chain_dma) { dev_warn(sdev->dev, "use_chain_dma set for scheduler %s", swidget->widget->name); - mutex_unlock(&ipc4_data->pipeline_state_mutex); - return 0; + goto unlock; }
header = SOF_IPC4_GLB_PIPE_INSTANCE_ID(swidget->instance_id); @@ -2326,7 +2325,7 @@ static int sof_ipc4_widget_free(struct snd_sof_dev *sdev, struct snd_sof_widget if (!pipeline->use_chain_dma) ida_free(&fw_module->m_ida, swidget->instance_id); } - +unlock: mutex_unlock(&ipc4_data->pipeline_state_mutex);
return ret; -- 2.41.0
On 6/10/23 06:36, Markus Elfring wrote:
From: Markus Elfring elfring@users.sourceforge.net Date: Sat, 10 Jun 2023 12:40:09 +0200
Add a jump target so that a call of the function “mutex_unlock” is stored only once in this function implementation.
Signed-off-by: Markus Elfring elfring@users.sourceforge.net
sound/soc/sof/ipc4-topology.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/sound/soc/sof/ipc4-topology.c b/sound/soc/sof/ipc4-topology.c index a4e1a70b607d..f0fd1dfa384e 100644 --- a/sound/soc/sof/ipc4-topology.c +++ b/sound/soc/sof/ipc4-topology.c @@ -2300,8 +2300,7 @@ static int sof_ipc4_widget_free(struct snd_sof_dev *sdev, struct snd_sof_widget if (pipeline->use_chain_dma) { dev_warn(sdev->dev, "use_chain_dma set for scheduler %s", swidget->widget->name);
mutex_unlock(&ipc4_data->pipeline_state_mutex);return 0;
goto unlock;}
header = SOF_IPC4_GLB_PIPE_INSTANCE_ID(swidget->instance_id);
@@ -2326,7 +2325,7 @@ static int sof_ipc4_widget_free(struct snd_sof_dev *sdev, struct snd_sof_widget if (!pipeline->use_chain_dma) ida_free(&fw_module->m_ida, swidget->instance_id); }
+unlock: mutex_unlock(&ipc4_data->pipeline_state_mutex);
return ret;
The change looks good but I am wondering if we need to print a dev_warn log which is already done on the sof_ipc4_widget_setup() path.
This seems redundant. Ranjani, can we simplify?
participants (4)
-
Jyri Sarha -
Mark Brown -
Markus Elfring -
Pierre-Louis Bossart