[PATCH 0/3] ASoC: SOF: debug: cleanups
cleanups of allocation and error handling
Guennadi Liakhovetski (3): ASoC: SOF: fix debugfs initialisation error handling ASoC: SOF: only allocate debugfs cache buffers for IPC flood entries ASoC: SOF: remove superfluous NULL check in debugfs read
sound/soc/sof/core.c | 5 +++-- sound/soc/sof/debug.c | 21 +++++++++++---------- 2 files changed, 14 insertions(+), 12 deletions(-)
From: Guennadi Liakhovetski guennadi.liakhovetski@linux.intel.com
If debugfs initialisation fails partially in sof_probe_continue() some debugfs files and the root directory might have been created successfully. They have to be cleaned up if some of them failed too.
Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Xiuli Pan xiulipan@outlook.com Signed-off-by: Guennadi Liakhovetski guennadi.liakhovetski@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/core.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c index 6d8f7d9fd192..495295a34c3a 100644 --- a/sound/soc/sof/core.c +++ b/sound/soc/sof/core.c @@ -154,7 +154,7 @@ static int sof_probe_continue(struct snd_sof_dev *sdev) if (ret < 0) { dev_err(sdev->dev, "error: failed to get machine info %d\n", ret); - goto dbg_err; + goto dsp_err; }
/* set up platform component driver */ @@ -257,8 +257,9 @@ static int sof_probe_continue(struct snd_sof_dev *sdev) fw_load_err: snd_sof_ipc_free(sdev); ipc_err: - snd_sof_free_debug(sdev); dbg_err: + snd_sof_free_debug(sdev); +dsp_err: snd_sof_remove(sdev);
/* all resources freed, update state to match */
From: Guennadi Liakhovetski guennadi.liakhovetski@linux.intel.com
snd_sof_debugfs_buf_item() is an exported function and is called from different locations to initialise different debugfs entries. However .cache_buf is only needed for IPC flood entries. Limit allocations respectively.
Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Xiuli Pan xiulipan@outlook.com Signed-off-by: Guennadi Liakhovetski guennadi.liakhovetski@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/debug.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/sound/soc/sof/debug.c b/sound/soc/sof/debug.c index 715a374b33cf..778c7d028493 100644 --- a/sound/soc/sof/debug.c +++ b/sound/soc/sof/debug.c @@ -609,14 +609,16 @@ int snd_sof_debugfs_buf_item(struct snd_sof_dev *sdev, dfse->sdev = sdev;
#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_IPC_FLOOD_TEST) - /* - * cache_buf is unused for SOF_DFSENTRY_TYPE_BUF debugfs entries. - * So, use it to save the results of the last IPC flood test. - */ - dfse->cache_buf = devm_kzalloc(sdev->dev, IPC_FLOOD_TEST_RESULT_LEN, - GFP_KERNEL); - if (!dfse->cache_buf) - return -ENOMEM; + if (!strncmp(name, "ipc_flood", strlen("ipc_flood"))) { + /* + * cache_buf is unused for SOF_DFSENTRY_TYPE_BUF debugfs entries. + * So, use it to save the results of the last IPC flood test. + */ + dfse->cache_buf = devm_kzalloc(sdev->dev, IPC_FLOOD_TEST_RESULT_LEN, + GFP_KERNEL); + if (!dfse->cache_buf) + return -ENOMEM; + } #endif
debugfs_create_file(name, mode, sdev->debugfs_root, dfse,
From: Guennadi Liakhovetski guennadi.liakhovetski@linux.intel.com
When reading from IPC flood debugfs entries no need to check whether .cache_buf is NULL - it's impossible since otherwise the initialisation would have failed. This also fixes a klocwork reported issue:
passed to function and may be dereferenced there by passing argument 2 to function 'memcpy' at line 510. sound/soc/sof/debug.c:510 | sof_dfsentry_read()
Reported-by: Keqiao Zhang keqiao.zhang@intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Xiuli Pan xiulipan@outlook.com Signed-off-by: Guennadi Liakhovetski guennadi.liakhovetski@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/soc/sof/debug.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/sound/soc/sof/debug.c b/sound/soc/sof/debug.c index 778c7d028493..a51a928ea40a 100644 --- a/sound/soc/sof/debug.c +++ b/sound/soc/sof/debug.c @@ -451,8 +451,7 @@ static ssize_t sof_dfsentry_read(struct file *file, char __user *buffer,
dentry = file->f_path.dentry; if ((!strcmp(dentry->d_name.name, "ipc_flood_count") || - !strcmp(dentry->d_name.name, "ipc_flood_duration_ms")) && - dfse->cache_buf) { + !strcmp(dentry->d_name.name, "ipc_flood_duration_ms"))) { if (*ppos) return 0;
On Mon, 15 Mar 2021 11:39:29 -0500, Pierre-Louis Bossart wrote:
cleanups of allocation and error handling
Guennadi Liakhovetski (3): ASoC: SOF: fix debugfs initialisation error handling ASoC: SOF: only allocate debugfs cache buffers for IPC flood entries ASoC: SOF: remove superfluous NULL check in debugfs read
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/3] ASoC: SOF: fix debugfs initialisation error handling commit: 11fd6d8e097b5b179ea445e0206aaefc47e62845 [2/3] ASoC: SOF: only allocate debugfs cache buffers for IPC flood entries commit: 72c35856b5edc3f734be5699e9f6737190a1d897 [3/3] ASoC: SOF: remove superfluous NULL check in debugfs read commit: 97f53046d746bef513d5fbaac53eedb011968407
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
participants (2)
-
Mark Brown -
Pierre-Louis Bossart