| Message ID | 20200902154229.1440489-1-kai.vehmanen@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ALSA: hda: release resource when snd_hdac_device_init is failed | expand |
On Wed, 02 Sep 2020 17:42:29 +0200, Kai Vehmanen wrote: > > From: Rander Wang <rander.wang@intel.com> > > When snd_hdac_device_init is failed, the codec is released by kfree > immediately without releasing some resources. The vendor_name should > be free if the memory is allocated and the runtime pm status should > be restored, especially the runtime pm status of parent device. It's released via put_device() and this should call the release callback, default_release(), and it contains all those kfree()'s and pm_runtime_*(). Could you double-check whether it's really missing? thanks, Takashi > > Signed-off-by: Rander Wang <rander.wang@intel.com> > Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com> > Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> > Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com> > Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com> > Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com> > --- > sound/hda/hdac_device.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/sound/hda/hdac_device.c b/sound/hda/hdac_device.c > index 3e9e9ac804f6..8d9d37225701 100644 > --- a/sound/hda/hdac_device.c > +++ b/sound/hda/hdac_device.c > @@ -109,12 +109,16 @@ int snd_hdac_device_init(struct hdac_device *codec, struct hdac_bus *bus, > codec->vendor_id & 0xffff); > if (!codec->chip_name) { > err = -ENOMEM; > - goto error; > + goto error_chip; > } > > return 0; > > + error_chip: > + kfree(codec->vendor_name); > error: > + pm_runtime_put_noidle(&codec->dev); > + pm_runtime_set_suspended(&codec->dev); > put_device(&codec->dev); > return err; > } > -- > 2.27.0 >
Hey, On Wed, 2 Sep 2020, Takashi Iwai wrote: > On Wed, 02 Sep 2020 17:42:29 +0200, Kai Vehmanen wrote: > > When snd_hdac_device_init is failed, the codec is released by kfree > > immediately without releasing some resources. The vendor_name should > > It's released via put_device() and this should call the release > callback, default_release(), and it contains all those kfree()'s and > pm_runtime_*(). > > Could you double-check whether it's really missing? ack, thanks for spotting. put_device() indeed calls the default release, so this patch is not correct. Rander, can you check as well this matches with the scenario you were looking at? Br, Kai
Thanks, I got the point, so the fix in snd_hdac_device_exit will resolve the issue completely. Rander > -----Original Message----- > From: Kai Vehmanen <kai.vehmanen@linux.intel.com> > Sent: Friday, September 4, 2020 7:31 PM > To: Takashi Iwai <tiwai@suse.de>; Wang, Rander <rander.wang@intel.com> > Cc: Kai Vehmanen <kai.vehmanen@linux.intel.com>; alsa-devel@alsa- > project.org; Ranjani Sridharan <ranjani.sridharan@linux.intel.com>; Pierre- > Louis Bossart <pierre-louis.bossart@linux.intel.com>; Bard Liao <yung- > chuan.liao@linux.intel.com>; Guennadi Liakhovetski > <guennadi.liakhovetski@linux.intel.com> > Subject: Re: [PATCH] ALSA: hda: release resource when snd_hdac_device_init is > failed > > Hey, > > On Wed, 2 Sep 2020, Takashi Iwai wrote: > > > On Wed, 02 Sep 2020 17:42:29 +0200, Kai Vehmanen wrote: > > > When snd_hdac_device_init is failed, the codec is released by kfree > > > immediately without releasing some resources. The vendor_name should > > > > It's released via put_device() and this should call the release > > callback, default_release(), and it contains all those kfree()'s and > > pm_runtime_*(). > > > > Could you double-check whether it's really missing? > > ack, thanks for spotting. put_device() indeed calls the default release, so this > patch is not correct. > > Rander, can you check as well this matches with the scenario you were looking > at? > > Br, Kai
diff --git a/sound/hda/hdac_device.c b/sound/hda/hdac_device.c index 3e9e9ac804f6..8d9d37225701 100644 --- a/sound/hda/hdac_device.c +++ b/sound/hda/hdac_device.c @@ -109,12 +109,16 @@ int snd_hdac_device_init(struct hdac_device *codec, struct hdac_bus *bus, codec->vendor_id & 0xffff); if (!codec->chip_name) { err = -ENOMEM; - goto error; + goto error_chip; } return 0; + error_chip: + kfree(codec->vendor_name); error: + pm_runtime_put_noidle(&codec->dev); + pm_runtime_set_suspended(&codec->dev); put_device(&codec->dev); return err; }