ASoC: hdac_hda: fix memleak on module unload #2252
Conversation
|
SOFCI TEST |
| if (codec->patch_ops.free) | ||
| codec->patch_ops.free(codec); | ||
|
|
||
| snd_hda_codec_cleanup_for_unbind(codec); |
There was a problem hiding this comment.
wow, this looks like a lot of clean up was missing? I see snd_hda_codec_cleanup_for_unbind() is currently called on the clean up path from hda_codec_driver_remove(). Is that function not called at all for this codec in the mem-leak case? Should it be called? Just for my information: how are these two clean up routines (codec and component .remove()) supposed to work together in this case?
There was a problem hiding this comment.
@lyakh yeah, maybe I should use stronger wording in the commit message -- we missed releasing 95% of allocated resources. I have looked at the cleanup routines many times, but it never occured to me to check that they are actually called... and it appears they are not. This is not so easy to pick-up with code review. hda_codec_driver_remove() is called but as ASoC/SOF sets the hdac-ext ops, the removal patch just calls "return codec->bus->core.ext_ops->hdev_detach(&codec->core);" For the latter question, component remove is called when the ASoC card is removed and codec remote is called later when the codec driver module is finally unloaded. The HDA codec remove functions have some bits that assume the parent card to be still around, so we have to do most of the the cleanup in component remove.
To avoid duplicated code for cleanup, and match the already exported snd_hda_codec_pcm_new(), also export snd_hda_codec_cleanup_for_unbind(). Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Fix a copy and paste error in snd_hda_codec_cleanup() documentation. Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Add error handling for patch_ops in hdac_hda_codec_probe(). Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
The hdac_hda remove implementation fails to free the hda codec resources, leading to memleaks at module unload. This gap has been there from the start, commit 6bae5ea ("ASoC: hdac_hda: add asoc extension for legacy HDA codec drivers"). Instead of duplicating the cleanup logic, use the common snd_hda_codec_cleanup_for_unbind() to free the resources. Remove existing code in hdac_hda to cleanup "codec.jackpoll_work" and call to snd_hdac_regmap_exit(), as these are already done in snd_hda_codec_cleanup_for_unbind(). The cleanup is done in ASoC component remove() callback and not in the HDAC bus hdev_detach(). This is done to ensure the codec specific cleanup routines are run before the parent card is freed. Fixes: 6bae5ea ("ASoC: hdac_hda: add asoc extension for legacy HDA codec drivers") BugLink: thesofproject#2195 Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
…manen <kai.vehmanen@linux.intel.com>: Hi, this small series is preparation for a set of bugfix ASoC patches addressing a memleak at module unload for the HDA codec wrapper. Instead of duplicating HDA code in ASoC tree, I chose to export more functionality from hda_codec.c so it can be (re)used in ASoC's hdac_hda.c. Full series: thesofproject#2252 Takashi and Mark, feedback is welcome on how to best handle this kind of series where I have dependent patches both in sound/pci/hda and in ASoC. For this series, I'm sending the patches separately and when/if first set is merged by Takashi, I'll route the ASoC patches via our usually SOF set to Mark. Kai Vehmanen (2): ALSA: hda: export snd_hda_codec_cleanup_for_unbind() ALSA: hda: fix snd_hda_codec_cleanup() documentation include/sound/hda_codec.h | 2 ++ sound/pci/hda/hda_codec.c | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) -- 2.27.0
Set of patches to use common cleanup code to free hda codec resources.
Fixes: #2195