[1/4] ASoC: soc-dapm.c: don't use kzalloc() for param on snd_soc_dai_link_event_pre_pmu()

Commit Message

Kuninori Morimoto Sept. 5, 2022, 11:17 p.m. UTC

From: Kuninori Morimoto <[e-mail]>

Current snd_soc_dai_link_event_pre_pmu() is using kzalloc() / kfree()
for "params", but it is fixed size, and not used on private data.
It is used for setup rtd at end of this function, just a local variable.
We don't need to use kzalloc() / kfree() for it.
This patch replace it as local variable.

Signed-off-by: Kuninori Morimoto <[e-mail]>
---
 sound/soc/soc-dapm.c | 38 +++++++++++++++-----------------------
 1 file changed, 15 insertions(+), 23 deletions(-)

Comments

Mark Brown Sept. 6, 2022, 12:02 p.m. UTC | #1

On Mon, Sep 05, 2022 at 11:17:29PM +0000, Kuninori Morimoto wrote:
> From: Kuninori Morimoto <[e-mail]>
> 
> Current snd_soc_dai_link_event_pre_pmu() is using kzalloc() / kfree()
> for "params", but it is fixed size, and not used on private data.
> It is used for setup rtd at end of this function, just a local variable.
> We don't need to use kzalloc() / kfree() for it.
> This patch replace it as local variable.

The reason we're allocating it dynamically is that it's quite large (608
bytes on arm64) and is starting to get a bit excessive for allocation on
the stack, especially when you're building with some of the KASAN type
stuff that increases stack usage.

Kuninori Morimoto Sept. 6, 2022, 11:52 p.m. UTC | #2

Hi Mark

> > Current snd_soc_dai_link_event_pre_pmu() is using kzalloc() / kfree()
> > for "params", but it is fixed size, and not used on private data.
> > It is used for setup rtd at end of this function, just a local variable.
> > We don't need to use kzalloc() / kfree() for it.
> > This patch replace it as local variable.
> 
> The reason we're allocating it dynamically is that it's quite large (608
> bytes on arm64) and is starting to get a bit excessive for allocation on
> the stack, especially when you're building with some of the KASAN type
> stuff that increases stack usage.

Oh, I see.
Thank you for clearing the reason.
I think it is very nice to have such comment on the code.

Thank you for your help !!

Best regards
---
Kuninori Morimoto

Patch

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index 73b8bd452ca7..7d4e4068f870 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -3837,15 +3837,13 @@  snd_soc_dai_link_event_pre_pmu(struct snd_soc_dapm_widget *w,
 	struct snd_soc_dapm_path *path;
 	struct snd_soc_dai *source, *sink;
 	struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
-	struct snd_pcm_hw_params *params = NULL;
+	struct snd_pcm_hw_params params;
 	const struct snd_soc_pcm_stream *config = NULL;
 	struct snd_pcm_runtime *runtime = NULL;
 	unsigned int fmt;
 	int ret = 0;
 
-	params = kzalloc(sizeof(*params), GFP_KERNEL);
-	if (!params)
-		return -ENOMEM;
+	memset(&params, 0, sizeof(params));
 
 	runtime = kzalloc(sizeof(*runtime), GFP_KERNEL);
 	if (!runtime) {
@@ -3902,45 +3900,39 @@  snd_soc_dai_link_event_pre_pmu(struct snd_soc_dapm_widget *w,
 		goto out;
 	}
 
-	snd_mask_set(hw_param_mask(params, SNDRV_PCM_HW_PARAM_FORMAT), fmt);
-	hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE)->min =
-		config->rate_min;
-	hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE)->max =
-		config->rate_max;
-	hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS)->min
-		= config->channels_min;
-	hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS)->max
-		= config->channels_max;
+	snd_mask_set(hw_param_mask(&params, SNDRV_PCM_HW_PARAM_FORMAT), fmt);
+	hw_param_interval(&params, SNDRV_PCM_HW_PARAM_RATE)->min	= config->rate_min;
+	hw_param_interval(&params, SNDRV_PCM_HW_PARAM_RATE)->max	= config->rate_max;
+	hw_param_interval(&params, SNDRV_PCM_HW_PARAM_CHANNELS)->min	= config->channels_min;
+	hw_param_interval(&params, SNDRV_PCM_HW_PARAM_CHANNELS)->max	= config->channels_max;
 
 	substream->stream = SNDRV_PCM_STREAM_CAPTURE;
 	snd_soc_dapm_widget_for_each_source_path(w, path) {
 		source = path->source->priv;
 
-		ret = snd_soc_dai_hw_params(source, substream, params);
+		ret = snd_soc_dai_hw_params(source, substream, &params);
 		if (ret < 0)
 			goto out;
 
-		dapm_update_dai_unlocked(substream, params, source);
+		dapm_update_dai_unlocked(substream, &params, source);
 	}
 
 	substream->stream = SNDRV_PCM_STREAM_PLAYBACK;
 	snd_soc_dapm_widget_for_each_sink_path(w, path) {
 		sink = path->sink->priv;
 
-		ret = snd_soc_dai_hw_params(sink, substream, params);
+		ret = snd_soc_dai_hw_params(sink, substream, &params);
 		if (ret < 0)
 			goto out;
 
-		dapm_update_dai_unlocked(substream, params, sink);
+		dapm_update_dai_unlocked(substream, &params, sink);
 	}
 
-	runtime->format = params_format(params);
-	runtime->subformat = params_subformat(params);
-	runtime->channels = params_channels(params);
-	runtime->rate = params_rate(params);
-
+	runtime->format		= params_format(&params);
+	runtime->subformat	= params_subformat(&params);
+	runtime->channels	= params_channels(&params);
+	runtime->rate		= params_rate(&params);
 out:
-	kfree(params);
 	return ret;
 }