[PATCH] ALSA: core: Warn on empty module

[Marek Szyprowski]

Hi

On 24.06.2020 18:03, Takashi Iwai wrote:
> The module argument passed to snd_card_new() must be a valid non-NULL
> pointer when the module support is enabled.  Since ASoC driver passes
> the argument from each snd_soc_card definition, one may forget to set
> the owner field and lead to a NULL module easily.
>
> For catching such an overlook, add a WARN_ON() in snd_card_new().
> Also, put the card->module assignment in the ifdef block for a very
> minor optimization.
>
> Signed-off-by: Takashi Iwai <tiwai at suse.de>

I know that this is intended, but I would like to note that this patch 
reveals the following issue on Raspberry Pi 3B with ARM 32bit kernel 
compiled from multi_v7_defconfig:

------------[ cut here ]------------
WARNING: CPU: 1 PID: 210 at sound/core/init.c:207 
snd_card_new+0x378/0x398 [snd]
Modules linked in: vc4(+) snd_soc_core ac97_bus snd_pcm_dmaengine 
bluetooth snd_pcm snd_timer crc32_arm_ce raspberrypi_hwmon snd soundcore 
ecdh_generic ecc bcm2835_thermal phy_generic
CPU: 1 PID: 210 Comm: systemd-udevd Not tainted 
5.8.0-rc1-00027-g81033c6b584b #1087
Hardware name: BCM2835
[<c03113c0>] (unwind_backtrace) from [<c030bcb4>] (show_stack+0x10/0x14)
[<c030bcb4>] (show_stack) from [<c071cef8>] (dump_stack+0xd4/0xe8)
[<c071cef8>] (dump_stack) from [<c0345bfc>] (__warn+0xdc/0xf4)
[<c0345bfc>] (__warn) from [<c0345cc4>] (warn_slowpath_fmt+0xb0/0xb8)
[<c0345cc4>] (warn_slowpath_fmt) from [<bf02ff74>] 
(snd_card_new+0x378/0x398 [snd])
[<bf02ff74>] (snd_card_new [snd]) from [<bf11f0b4>] 
(snd_soc_bind_card+0x280/0x99c [snd_soc_core])
[<bf11f0b4>] (snd_soc_bind_card [snd_soc_core]) from [<bf12f000>] 
(devm_snd_soc_register_card+0x34/0x6c [snd_soc_core])
[<bf12f000>] (devm_snd_soc_register_card [snd_soc_core]) from 
[<bf165654>] (vc4_hdmi_bind+0x43c/0x5f4 [vc4])
[<bf165654>] (vc4_hdmi_bind [vc4]) from [<c09d660c>] 
(component_bind_all+0xec/0x24c)
[<c09d660c>] (component_bind_all) from [<bf15c44c>] 
(vc4_drm_bind+0xd4/0x174 [vc4])
[<bf15c44c>] (vc4_drm_bind [vc4]) from [<c09d6ac0>] 
(try_to_bring_up_master+0x160/0x1b0)
[<c09d6ac0>] (try_to_bring_up_master) from [<c09d6f38>] 
(component_master_add_with_match+0xd0/0x104)
[<c09d6f38>] (component_master_add_with_match) from [<bf15c588>] 
(vc4_platform_drm_probe+0x9c/0xbc [vc4])
[<bf15c588>] (vc4_platform_drm_probe [vc4]) from [<c09df740>] 
(platform_drv_probe+0x6c/0xa4)
[<c09df740>] (platform_drv_probe) from [<c09dd6f0>] 
(really_probe+0x210/0x350)
[<c09dd6f0>] (really_probe) from [<c09dd940>] 
(driver_probe_device+0x5c/0xb4)
[<c09dd940>] (driver_probe_device) from [<c09ddb38>] 
(device_driver_attach+0x58/0x60)
[<c09ddb38>] (device_driver_attach) from [<c09ddbc0>] 
(__driver_attach+0x80/0xbc)
[<c09ddbc0>] (__driver_attach) from [<c09db820>] 
(bus_for_each_dev+0x68/0xb4)
[<c09db820>] (bus_for_each_dev) from [<c09dc9f8>] 
(bus_add_driver+0x130/0x1e8)
[<c09dc9f8>] (bus_add_driver) from [<c09de648>] (driver_register+0x78/0x110)
[<c09de648>] (driver_register) from [<c0302038>] 
(do_one_initcall+0x50/0x220)
[<c0302038>] (do_one_initcall) from [<c03db544>] (do_init_module+0x60/0x210)
[<c03db544>] (do_init_module) from [<c03da4f8>] (load_module+0x1e34/0x2338)
[<c03da4f8>] (load_module) from [<c03dac00>] (sys_finit_module+0xac/0xbc)
[<c03dac00>] (sys_finit_module) from [<c03000c0>] 
(ret_fast_syscall+0x0/0x54)
Exception stack(0xeded9fa8 to 0xeded9ff0)
...
---[ end trace 6414689569c2bc08 ]---

This warning is not present when booting ARM 64bit kernel, but I suspect 
that this is due to the differences in the kernel configuration.

> ---
>   sound/core/init.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/sound/core/init.c b/sound/core/init.c
> index b02a99766351..0478847ba2b8 100644
> --- a/sound/core/init.c
> +++ b/sound/core/init.c
> @@ -203,7 +203,10 @@ int snd_card_new(struct device *parent, int idx, const char *xid,
>   	mutex_unlock(&snd_card_mutex);
>   	card->dev = parent;
>   	card->number = idx;
> +#ifdef MODULE
> +	WARN_ON(!module);
>   	card->module = module;
> +#endif
>   	INIT_LIST_HEAD(&card->devices);
>   	init_rwsem(&card->controls_rwsem);
>   	rwlock_init(&card->ctl_files_rwlock);

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland