Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ASoC: soc-core: fix kernel oops on dai link remove #1469

Closed

Conversation

plbossart
Copy link
Member

soc_dai_link_remove() can be called from topology or on cleanup the
card. The kernel oopses if soc_unbind_dai_link() is called in the
first case, so add a boolean parameter and conditionally call
soc_unbind_dai_link()

Fixes: bc7a909 ("ASoC: soc-core: add soc_unbind_dai_link()")
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com

Fixes: #1466

soc_dai_link_remove() can be called from topology or on cleanup the
card. The kernel oopses if soc_unbind_dai_link() is called in the
first case, so add a boolean parameter and conditionally call
soc_unbind_dai_link()

Fixes: bc7a909 ("ASoC: soc-core: add soc_unbind_dai_link()")
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
@plbossart
Copy link
Member Author

@morimoto @ranj063 this patch seems to avoid the kernel oops, but I am not really happy about it.
a) I am not sure why we should do something specific about topology-defined dailinks?
b) reverting the original patch is actually simpler.

@@ -1997,7 +2000,7 @@ static void soc_cleanup_card_resources(struct snd_soc_card *card)
soc_remove_link_dais(card);

for_each_card_links_safe(card, link, _link)
snd_soc_remove_dai_link(card, link);
snd_soc_remove_dai_link(card, link, true);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@plbossart isnt the simpler thing to just remove soc_unbind_dai_link() from remove_dai_link() and add it after the call to soc_unbind_dai_link()?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that's what the code initially did, so that's essentially a revert, yes.

@keyonjie
Copy link

I still seeing errors like this either with this commit applied or Morimoto's commit reverted:

[  226.344251] INFO: trying to register non-static key.
[  226.344260] the code is fine but needs lockdep annotation.
[  226.344263] turning off the locking correctness validator.
[  226.344269] CPU: 0 PID: 5220 Comm: rmmod Not tainted 5.4.0-rc7+ #5
[  226.344272] Hardware name: GOOGLE Cyan/Cyan, BIOS MrChromebox-4.10 08/22/2019
[  226.344275] Call Trace:
[  226.344286]  dump_stack+0x71/0xa0
[  226.344294]  register_lock_class+0x549/0x550
[  226.344300]  __lock_acquire+0x71/0x1910
[  226.344307]  ? __mutex_lock+0xa0/0x970
[  226.344311]  lock_acquire+0x95/0x190
[  226.344317]  ? init_timer_key+0x110/0x110
[  226.344322]  del_timer_sync+0x2f/0x90
[  226.344326]  ? init_timer_key+0x110/0x110
[  226.344332]  flush_delayed_work+0x13/0x40
[  226.344355]  soc_pcm_private_free+0x17/0x20 [snd_soc_core]
[  226.344368]  snd_pcm_free+0x1a/0x50 [snd_pcm]
[  226.344379]  __snd_device_free+0x46/0x60 [snd]
[  226.344387]  snd_device_free_all+0x3a/0x70 [snd]
[  226.344395]  release_card_device+0x14/0x50 [snd]
[  226.344401]  device_release+0x23/0x80
[  226.344406]  kobject_put+0x84/0x1a0
[  226.344414]  snd_card_free+0x5b/0x90 [snd]
[  226.344430]  soc_cleanup_card_resources+0xbb/0x2c0 [snd_soc_core]
[  226.344445]  snd_soc_unbind_card+0x9f/0xe0 [snd_soc_core]
[  226.344459]  snd_soc_unregister_card+0x1f/0x60 [snd_soc_core]
[  226.344465]  release_nodes+0x1ad/0x200
[  226.344471]  device_release_driver_internal+0xef/0x1c0
[  226.344475]  bus_remove_device+0xed/0x160
[  226.344479]  device_del+0x15e/0x370
[  226.344484]  platform_device_del.part.0+0xe/0x60
[  226.344488]  platform_device_unregister+0x17/0x30
[  226.344499]  snd_sof_device_remove+0x4c/0xa0 [snd_sof]
[  226.344505]  sof_acpi_remove+0x2f/0x40 [snd_sof_acpi]
[  226.344509]  platform_drv_remove+0x1f/0x40
[  226.344514]  device_release_driver_internal+0xdf/0x1c0
[  226.344518]  driver_detach+0x42/0x80
[  226.344522]  bus_remove_driver+0x56/0xca
[  226.344527]  __x64_sys_delete_module+0x18f/0x240
[  226.344533]  ? trace_hardirqs_off_thunk+0x1a/0x20
[  226.344537]  do_syscall_64+0x4b/0x1b0
[  226.344541]  entry_SYSCALL_64_after_hwframe+0x49/0xbe
[  226.344546] RIP: 0033:0x7f986e3c01b7
[  226.344551] Code: 73 01 c3 48 8b 0d d1 8c 2c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 b8 b0 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d a1 8c 2c 00 f7 d8 64 89 01 48
[  226.344554] RSP: 002b:00007ffde9e5a6e8 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
[  226.344560] RAX: ffffffffffffffda RBX: 00007ffde9e5a748 RCX: 00007f986e3c01b7
[  226.344562] RDX: 000000000000000a RSI: 0000000000000800 RDI: 00005628ea7ec7d8
[  226.344565] RBP: 00005628ea7ec770 R08: 00007ffde9e59661 R09: 0000000000000000
[  226.344568] R10: 00007f986e43ccc0 R11: 0000000000000206 R12: 00007ffde9e5a910
[  226.344571] R13: 00007ffde9e5b620 R14: 00005628ea7ec260 R15: 00005628ea7ec770

Can you help me to understand what it do happen and give a clean fix for it?

@plbossart
Copy link
Member Author

well, this is on Cyan so not sure if it ever worked.

Try bisecting with sof-dev-rebase-2019xxxx tags to see if the recent changes to the core changed stuff. I went as far as 20190821 in some cases...

plbossart added a commit to plbossart/sound that referenced this pull request Nov 15, 2019
The initial change in thesofproject#1469
does not work on Baytrail.

This fix suggested on November 13, 2019 by Morimoto-san seems to fix
the issues.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
@plbossart
Copy link
Member Author

This PR is not good enough on Baytrail/Cherrytrail, let's try PR #1504

@plbossart plbossart closed this Nov 15, 2019
plbossart added a commit that referenced this pull request Nov 16, 2019
The initial change in #1469
does not work on Baytrail.

This fix suggested on November 13, 2019 by Morimoto-san seems to fix
the issues.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kernel oops during sof_bootloop.sh test
3 participants