Re: [alsa-devel] [PATCH] ASoC: dapm: fix module unload/reload failed issue by reset kcontrol's widget list

-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Tuesday, July 28, 2015 5:07 PM To: Jie, Yang Cc: broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R Subject: Re: [PATCH] ASoC: dapm: fix module unload/reload failed issue by reset kcontrol's widget list

On Tue, 28 Jul 2015 11:03:39 +0200, Jie Yang wrote:

...

We need reset the related kcontrol's widget list during the freeing of a widget, otherwise, the widget list may be outdated and reference to it may introduce errors(they usually occurs during driver modules unloading/reloading).

Here adding a func dapm_update_kcontrols_of_freeing_widget and call it at dapm_update_kcontrols_of_freeing_widget, to fix those issues.

Signed-off-by: Jie Yang yang.jie@intel.com

sound/soc/soc-dapm.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index aa327c9..1ad8144 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -2305,6 +2305,35 @@ static void dapm_free_path(struct

snd_soc_dapm_path *path)

...

kfree(path); }

+/* update all dapm kcontrols that related to a widget which being +freed*/ static void dapm_update_kcontrols_of_freeing_widget(struct +snd_soc_dapm_widget * w) {

• struct dapm_kcontrol_data *data = NULL;
• int i, j, n;
• if (w && w->kcontrols) {

• for (i = 0; i < w->num_kcontrols; i++) {
  

• 	if (w->kcontrols[i] == NULL)
  

• 		continue;
  

• 	data = snd_kcontrol_chip(w->kcontrols[i]);
  

• 	if (data == NULL)
  

• 		continue;
  

• 	for (j = 0, n = 0; j < data->wlist->num_widgets; j++) {
  

• 		if (data->wlist->widgets[j] == w) {
  

• 			data->wlist->widgets[j] = NULL;
  

• 			n++;
  

• 		}
  

• 	}
  

• 	if (n) {
  

• 		data->wlist->num_widgets -= n;
  

This looks buggy. You didn't rearrange widgets[] but just put a hole, yet decreasing the number.

You are right, but since krealloc() cannot shrink the allocated size, maybe we should free those related kcontrols here?

BTW, how does the reference occur while unloading? Or better to say: isn't it the module refcount adjusted while accessing the resources?

soc_cleanup_card_resources snd_soc_dapm_free dapm_free_widgets snd_card_free //removing kcontrols, calling private dapm_kcontrol_free() kfree(data->widget->name);//the widget here may be //already freed! So the reference here //may cause error.

So, actually, here resetting data->widget to NULL may be what we really need, and updating data->wlist (num_widgets and widgets[j]) is only the icing I wanted on the cake, maybe I should remove it?

Thanks, ~Keyon

thanks,

Takashi