Hey,
On Mon, 17 Feb 2020, Takashi Iwai wrote:
On Mon, 17 Feb 2020 15:21:37 +0100, Kai Vehmanen wrote:
at least in my tests, acomp_exit() ended up releasing a bunch of devres allocations made with devm_kzalloc() that had no relation to acomp. I noted that in devres.c:find_group(), docs say:
Isn't it the devres object allocated in the component base code itself?
true, but problem are other devres allocations done to the PCI device.
I did some more testing and if I force a failure in snd_hdac_i915_init() and do cleanup with snd_hdac_acomp_exit() in the same function, it works as expected:
[ 1476.111313] sof-audio-pci 0000:00:1f.3: bound 0000:00:02.0 (ops vgt_balloon_space [i915]) [ 1476.111315] sof-audio-pci 0000:00:1f.3: couldn't bind with audio component [ 1476.111338] i915 0000:00:02.0: DEVRES REL 00000000b76305ff grp< (0 bytes) [ 1476.111346] sof-audio-pci 0000:00:1f.3: DEVRES REL 000000006f2e9c46 grp< (0 bytes) [ 1476.111360] sof-audio-pci 0000:00:1f.3: DEVRES REM 000000000810c760 hdac_acomp_release (64 bytes) [ 1476.111366] sof-audio-pci 0000:00:1f.3: init of i915 and HDMI codec failed
So this is good. It released the group created for acomp. But, but, if the init is successful, but later in probe I end up disabling i915 support (e.g. no codec driver in the kernel) and try to do cleanup of acomp resources by calling snd_hdac_i915_exit(), I get this suprising devres log:
[ 3007.592029] i915 0000:00:02.0: DEVRES REL 0000000071c16ac0 grp< (0 bytes) [ 3007.592038] sof-audio-pci 0000:00:1f.3: DEVRES REL 000000008b5ea24e grp< (0 bytes) [ 3007.592044] sof-audio-pci 0000:00:1f.3: DEVRES REL 00000000f9d4d35c devm_kzalloc_release (1816 bytes) [ 3007.592051] sof-audio-pci 0000:00:1f.3: DEVRES REL 00000000fa1e4d1a devm_kzalloc_release (1816 bytes) [ 3007.592057] sof-audio-pci 0000:00:1f.3: DEVRES REL 0000000057d8edec devm_kzalloc_release (320 bytes) [ 3007.592063] sof-audio-pci 0000:00:1f.3: DEVRES REL 00000000aea01a3d devm_kzalloc_release (320 bytes) [ 3007.592069] sof-audio-pci 0000:00:1f.3: DEVRES REL 0000000039d17832 devm_kzalloc_release (320 bytes) [ 3007.592075] sof-audio-pci 0000:00:1f.3: DEVRES REL 000000009958e500 devm_kzalloc_release (320 bytes) [ 3007.592081] sof-audio-pci 0000:00:1f.3: DEVRES REL 00000000eea2922f devm_kzalloc_release (320 bytes) [ 3007.592088] sof-audio-pci 0000:00:1f.3: DEVRES REL 00000000dd6f6f2a devm_kzalloc_release (320 bytes) [ 3007.592093] sof-audio-pci 0000:00:1f.3: DEVRES REL 000000005ca30f2d devm_kzalloc_release (320 bytes) [ 3007.592099] sof-audio-pci 0000:00:1f.3: DEVRES REL 00000000bf594d2d devm_kzalloc_release (320 bytes) [ 3007.592104] sof-audio-pci 0000:00:1f.3: DEVRES REL 000000009a0669c2 devm_kzalloc_release (320 bytes) [ 3007.592110] sof-audio-pci 0000:00:1f.3: DEVRES REL 00000000757c42ee devm_kzalloc_release (320 bytes) [ 3007.592115] sof-audio-pci 0000:00:1f.3: DEVRES REL 00000000a928376b devm_kzalloc_release (320 bytes) [ 3007.592121] sof-audio-pci 0000:00:1f.3: DEVRES REL 000000007bf93e19 devm_kzalloc_release (320 bytes) [ 3007.592127] sof-audio-pci 0000:00:1f.3: DEVRES REL 00000000485f5b74 devm_kzalloc_release (320 bytes) [ 3007.592133] sof-audio-pci 0000:00:1f.3: DEVRES REL 000000007c95c553 devm_kzalloc_release (320 bytes) [ 3007.592139] sof-audio-pci 0000:00:1f.3: DEVRES REL 00000000d7b60566 devm_kzalloc_release (320 bytes) [ 3007.592145] sof-audio-pci 0000:00:1f.3: DEVRES REL 000000002805a826 devm_kzalloc_release (320 bytes) [ 3007.592160] sof-audio-pci 0000:00:1f.3: DEVRES REM 0000000085720a3d hdac_acomp_release (64 bytes)
These allocations are not done for acomp, but are rather allocs done for the SOF DSP device during the init. E.g. the 1816 byte alloc is devm_kzalloc of "struct hdac_hda_priv":
[ 3007.558102] sof-audio-pci 0000:00:1f.3: HDA codec #0 probed OK: response: 10ec0700 [ 3007.558109] sof-audio-pci 0000:00:1f.3: DEVRES ADD 00000000fa1e4d1a devm_kzalloc_release (1816 bytes)
... now I guess this is just how the component framework works. We do register the hdac-bus device as aggregate master device in snd_hdac_acomp_init() by calling component_master_add_with_match(). So calling snd_hdac_acomp_exit() will bring down the aggregate master device.
I now worked around this my putting the acomp_exit() at end of the SOF DSP device removal.
Br, Kai