[PATCH 1/9] ALSA: hda: Do not unset preset when cleaning up codec

[Takashi Iwai]

On Tue, 12 Jul 2022 11:42:56 +0200,
Cezary Rojewski wrote:
> 
> On 2022-07-11 4:12 PM, Takashi Iwai wrote:
> > On Mon, 11 Jul 2022 10:25:17 +0200,
> > Cezary Rojewski wrote:
> 
> ...
> 
> >> avs-driver i.e. the bus driver takes responsibility for the codec
> >> device only. There is no real probe(), just the device creation and
> >> initialization of its fields. The rest is handled by the component
> >> driver (sound/soc/hda.c). If this field is cleared and the test is
> >> limited to reloading HDAudio codec module alone, we get a
> >> panic. Something similar to the stack found below my message.
> >> 
> >> In regard to the other question - are presets freed at all? It seems
> >> all of them are part of the static device-driver matching list. If so,
> >> the pointer is always valid.
> > 
> > When the codec driver is unbound and the module is unloaded, the whole
> > objects and symbols are gone.
> 
> 
> hda_codec_driver_remove() won't get even called when soc-card is being
> unbound so everything is still here.
> 
> >> [  136.827856] RIP: 0010:hda_codec_probe+0x16c/0x560 [snd_soc_hda_codec]
> 
> >> [  136.828568] Call Trace:
> >> [  136.828593]  <TASK>
> >> [  136.828628]  snd_soc_component_probe+0x3a/0x60 [snd_soc_core]
> >> [  136.828981]  soc_probe_component+0x276/0x4a0 [snd_soc_core]
> >> [  136.829274]  snd_soc_bind_card+0x819/0x13d0 [snd_soc_core]
> >> [  136.829560]  ? __kasan_slab_alloc+0x32/0x90
> >> [  136.829614]  snd_soc_register_card+0x24e/0x260 [snd_soc_core]
> >> [  136.829900]  devm_snd_soc_register_card+0x48/0x90 [snd_soc_core]
> >> [  136.830204]  avs_hdaudio_probe+0x298/0x2c0 [snd_soc_avs_hdaudio]
> 
> >> (...)
> > 
> > Hmm, in the Oops above, at which moment,
> > snd_hda_codec_cleanup_for_unbind() is called via which function?
> > Is it the unload of HD-audio codec driver during the probe of AVS
> > HD-audio?
> > 
> > The preset is assigned to the given HD-audio device object for the
> > attached codec driver.  Once after the codec driver gets unbound, you
> > must not access to this codec driver's methods any longer, hence we
> > clear the preset field.
> > 
> > So I wonder how the access to the codec->preset happens after the
> > codec unbind.
> 
> 
> Test scenario:
> - enumerate avs-driver stack on machine with HDAudio codec present
> - rmmod snd_soc_avs_hdaudio // just the machine board driver
> i.e. soc-card driver
> - modprobe snd_soc_avs_hdaudio
> >>> panic <<<
> 
> snd_hda_codec_cleanup_for_unbind() is called in more places than just
> HDAudio codec driver's probe() and remove(). It's also called whenever
> HDAudio codec soc-component is being removed. Relevant part of the
> stack showing when does the cleanup function get called during rmmod:
> 
> [  220.549349]  snd_hda_codec_cleanup_for_unbind+0x25/0x451 [snd_hda_codec]
> [  220.549536]  ? dump_stack_lvl+0x45/0x49
> [  220.549568]  hda_codec_remove.cold+0x14/0x138 [snd_soc_hda_codec]
> [  220.549609]  snd_soc_component_remove+0x34/0x40 [snd_soc_core]
> [  220.549942]  soc_remove_component+0x113/0x120 [snd_soc_core]
> [  220.550249]  soc_cleanup_card_resources+0x1a7/0x4a0 [snd_soc_core]
> [  220.550561]  snd_soc_unbind_card+0x9e/0x190 [snd_soc_core]
> [  220.550885]  snd_soc_unregister_card+0x28/0x80 [snd_soc_core]
> [  220.551193]  devm_card_release+0x1d/0x20 [snd_soc_core]
> [  220.551527]  release_nodes+0x73/0x170
> [  220.551549]  ? preempt_count_sub+0x18/0xc0
> [  220.551576]  devres_release_all+0x10a/0x150
> [  220.551600]  ? devres_remove_group+0x260/0x260
> [  220.551630]  device_unbind_cleanup+0x14/0xd0
> [  220.551656]  device_release_driver_internal+0x146/0x1d0
> [  220.551688]  driver_detach+0x81/0xf0
> [  220.551716]  bus_remove_driver+0xae/0x170
> [  220.551743]  driver_unregister+0x4d/0x70
> [  220.551770]  platform_driver_unregister+0x12/0x20
> [  220.551799]  avs_hdaudio_driver_exit+0x10/0x12 [snd_soc_avs_hdaudio]

So, IMO,  you're scratching a wrong surface.  The problem is rather
that snd_hda_codec_cleanup_for_unbind() is called even if it's not for
unbinding the codec.


Takashi