| Message ID | 20221117080823.77549-1-yuancan@huawei.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 5a2d96623670155d94aca72c320c0ac27bdc6bd2 |
| Headers | show |
| Series | platform/chrome: cros_usbpd_notify: Fix error handling in cros_usbpd_notify_init() | expand |
Hi Yuan, On Thu, Nov 17, 2022 at 12:10 AM Yuan Can <yuancan@huawei.com> wrote: > > The following WARNING message was given when rmmod cros_usbpd_notify: > > Unexpected driver unregister! > WARNING: CPU: 0 PID: 253 at drivers/base/driver.c:270 driver_unregister+0x8a/0xb0 > Modules linked in: cros_usbpd_notify(-) > CPU: 0 PID: 253 Comm: rmmod Not tainted 6.1.0-rc3 #24 > ... > Call Trace: > <TASK> > cros_usbpd_notify_exit+0x11/0x1e [cros_usbpd_notify] > __x64_sys_delete_module+0x3c7/0x570 > ? __ia32_sys_delete_module+0x570/0x570 > ? lock_is_held_type+0xe3/0x140 > ? syscall_enter_from_user_mode+0x17/0x50 > ? rcu_read_lock_sched_held+0xa0/0xd0 > ? syscall_enter_from_user_mode+0x1c/0x50 > do_syscall_64+0x37/0x90 > entry_SYSCALL_64_after_hwframe+0x63/0xcd > RIP: 0033:0x7f333fe9b1b7 > > The reason is that the cros_usbpd_notify_init() does not check the return > value of platform_driver_register(), and the cros_usbpd_notify can > install successfully even if platform_driver_register() failed. > > Fix by checking the return value of platform_driver_register() and Can you provide details regarding why cros_usbpd_notify_acpi_driver registration is failing on your system (logs or other details) ? Thanks, - Prashant
On Thu, Nov 17, 2022 at 08:08:23AM +0000, Yuan Can wrote: > The following WARNING message was given when rmmod cros_usbpd_notify: > > Unexpected driver unregister! > WARNING: CPU: 0 PID: 253 at drivers/base/driver.c:270 driver_unregister+0x8a/0xb0 > Modules linked in: cros_usbpd_notify(-) > CPU: 0 PID: 253 Comm: rmmod Not tainted 6.1.0-rc3 #24 > ... > Call Trace: > <TASK> > cros_usbpd_notify_exit+0x11/0x1e [cros_usbpd_notify] > __x64_sys_delete_module+0x3c7/0x570 > ? __ia32_sys_delete_module+0x570/0x570 > ? lock_is_held_type+0xe3/0x140 > ? syscall_enter_from_user_mode+0x17/0x50 > ? rcu_read_lock_sched_held+0xa0/0xd0 > ? syscall_enter_from_user_mode+0x1c/0x50 > do_syscall_64+0x37/0x90 > entry_SYSCALL_64_after_hwframe+0x63/0xcd > RIP: 0033:0x7f333fe9b1b7 > > The reason is that the cros_usbpd_notify_init() does not check the return > value of platform_driver_register(), and the cros_usbpd_notify can > install successfully even if platform_driver_register() failed. > > Fix by checking the return value of platform_driver_register() and > unregister cros_usbpd_notify_plat_driver when it failed. On first glance, the last part looked wrong (if register() fails, we don't need to call unregister()), but these are two different drivers getting registered. Looks good to me, then. > Fixes: ec2daf6e33f9 ("platform: chrome: Add cros-usbpd-notify driver") > Signed-off-by: Yuan Can <yuancan@huawei.com> Reviewed-by: Brian Norris <briannorris@chromium.org> > --- > drivers/platform/chrome/cros_usbpd_notify.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/platform/chrome/cros_usbpd_notify.c b/drivers/platform/chrome/cros_usbpd_notify.c > index 4b5a81c9dc6d..10670b6588e3 100644 > --- a/drivers/platform/chrome/cros_usbpd_notify.c > +++ b/drivers/platform/chrome/cros_usbpd_notify.c > @@ -239,7 +239,11 @@ static int __init cros_usbpd_notify_init(void) > return ret; > > #ifdef CONFIG_ACPI > - platform_driver_register(&cros_usbpd_notify_acpi_driver); > + ret = platform_driver_register(&cros_usbpd_notify_acpi_driver); > + if (ret) { > + platform_driver_unregister(&cros_usbpd_notify_plat_driver); > + return ret; > + } > #endif > return 0; > } > -- > 2.17.1 > >
在 2022/11/18 3:07, Prashant Malani 写道: > Hi Yuan, > > On Thu, Nov 17, 2022 at 12:10 AM Yuan Can <yuancan@huawei.com> wrote: >> The following WARNING message was given when rmmod cros_usbpd_notify: >> >> Unexpected driver unregister! >> WARNING: CPU: 0 PID: 253 at drivers/base/driver.c:270 driver_unregister+0x8a/0xb0 >> Modules linked in: cros_usbpd_notify(-) >> CPU: 0 PID: 253 Comm: rmmod Not tainted 6.1.0-rc3 #24 >> ... >> Call Trace: >> <TASK> >> cros_usbpd_notify_exit+0x11/0x1e [cros_usbpd_notify] >> __x64_sys_delete_module+0x3c7/0x570 >> ? __ia32_sys_delete_module+0x570/0x570 >> ? lock_is_held_type+0xe3/0x140 >> ? syscall_enter_from_user_mode+0x17/0x50 >> ? rcu_read_lock_sched_held+0xa0/0xd0 >> ? syscall_enter_from_user_mode+0x1c/0x50 >> do_syscall_64+0x37/0x90 >> entry_SYSCALL_64_after_hwframe+0x63/0xcd >> RIP: 0033:0x7f333fe9b1b7 >> >> The reason is that the cros_usbpd_notify_init() does not check the return >> value of platform_driver_register(), and the cros_usbpd_notify can >> install successfully even if platform_driver_register() failed. >> >> Fix by checking the return value of platform_driver_register() and > Can you provide details regarding why cros_usbpd_notify_acpi_driver registration > is failing on your system (logs or other details) ? The reason could be an OOM happend when platform_driver_register() tries to allocate memory, a simple call graph: platform_driver_register() driver_register() bus_add_driver() dev = kzalloc(...) # OOM happened
On Fri, Nov 18, 2022 at 2:18 AM Yuan Can <yuancan@huawei.com> wrote: > > > 在 2022/11/18 3:07, Prashant Malani 写道: > > Hi Yuan, > > > > On Thu, Nov 17, 2022 at 12:10 AM Yuan Can <yuancan@huawei.com> wrote: > >> The reason is that the cros_usbpd_notify_init() does not check the return > >> value of platform_driver_register(), and the cros_usbpd_notify can > >> install successfully even if platform_driver_register() failed. > >> > >> Fix by checking the return value of platform_driver_register() and > > Can you provide details regarding why cros_usbpd_notify_acpi_driver registration > > is failing on your system (logs or other details) ? > > The reason could be an OOM happend when platform_driver_register() tries > to allocate memory, > > a simple call graph: > > platform_driver_register() > driver_register() > bus_add_driver() > dev = kzalloc(...) # OOM happened > Sure, but is that what you're seeing? Can you share logs from your repro case? I've got no problems with the change itself, but find the platform_driver_register() failure to be quite odd here. Thanks,
在 2022/11/19 4:11, Prashant Malani 写道: > On Fri, Nov 18, 2022 at 2:18 AM Yuan Can <yuancan@huawei.com> wrote: >> >> 在 2022/11/18 3:07, Prashant Malani 写道: >>> Hi Yuan, >>> >>> On Thu, Nov 17, 2022 at 12:10 AM Yuan Can <yuancan@huawei.com> wrote: >>>> The reason is that the cros_usbpd_notify_init() does not check the return >>>> value of platform_driver_register(), and the cros_usbpd_notify can >>>> install successfully even if platform_driver_register() failed. >>>> >>>> Fix by checking the return value of platform_driver_register() and >>> Can you provide details regarding why cros_usbpd_notify_acpi_driver registration >>> is failing on your system (logs or other details) ? >> The reason could be an OOM happend when platform_driver_register() tries >> to allocate memory, >> >> a simple call graph: >> >> platform_driver_register() >> driver_register() >> bus_add_driver() >> dev = kzalloc(...) # OOM happened >> > Sure, but is that what you're seeing? Can you share logs from your repro case? Yes, that the reason, this problem is triggered by the OOM fault-injection test.
On Wed, Nov 23, 2022 at 12:33 AM Yuan Can <yuancan@huawei.com> wrote: > > > 在 2022/11/19 4:11, Prashant Malani 写道: > > On Fri, Nov 18, 2022 at 2:18 AM Yuan Can <yuancan@huawei.com> wrote: > >> > >> 在 2022/11/18 3:07, Prashant Malani 写道: > >>> Hi Yuan, > >>> > >>> On Thu, Nov 17, 2022 at 12:10 AM Yuan Can <yuancan@huawei.com> wrote: > >>>> The reason is that the cros_usbpd_notify_init() does not check the return > >>>> value of platform_driver_register(), and the cros_usbpd_notify can > >>>> install successfully even if platform_driver_register() failed. > >>>> > >>>> Fix by checking the return value of platform_driver_register() and > >>> Can you provide details regarding why cros_usbpd_notify_acpi_driver registration > >>> is failing on your system (logs or other details) ? > >> The reason could be an OOM happend when platform_driver_register() tries > >> to allocate memory, > >> > >> a simple call graph: > >> > >> platform_driver_register() > >> driver_register() > >> bus_add_driver() > >> dev = kzalloc(...) # OOM happened > >> > > Sure, but is that what you're seeing? Can you share logs from your repro case? > Yes, that the reason, this problem is triggered by the OOM > fault-injection test. Alright. Thanks for providing that detail. Best regards, -Prashant
Hello: This patch was applied to chrome-platform/linux.git (for-kernelci) by Prashant Malani <pmalani@chromium.org>: On Thu, 17 Nov 2022 08:08:23 +0000 you wrote: > The following WARNING message was given when rmmod cros_usbpd_notify: > > Unexpected driver unregister! > WARNING: CPU: 0 PID: 253 at drivers/base/driver.c:270 driver_unregister+0x8a/0xb0 > Modules linked in: cros_usbpd_notify(-) > CPU: 0 PID: 253 Comm: rmmod Not tainted 6.1.0-rc3 #24 > ... > Call Trace: > <TASK> > cros_usbpd_notify_exit+0x11/0x1e [cros_usbpd_notify] > __x64_sys_delete_module+0x3c7/0x570 > ? __ia32_sys_delete_module+0x570/0x570 > ? lock_is_held_type+0xe3/0x140 > ? syscall_enter_from_user_mode+0x17/0x50 > ? rcu_read_lock_sched_held+0xa0/0xd0 > ? syscall_enter_from_user_mode+0x1c/0x50 > do_syscall_64+0x37/0x90 > entry_SYSCALL_64_after_hwframe+0x63/0xcd > RIP: 0033:0x7f333fe9b1b7 > > [...] Here is the summary with links: - platform/chrome: cros_usbpd_notify: Fix error handling in cros_usbpd_notify_init() https://git.kernel.org/chrome-platform/c/5a2d96623670 You are awesome, thank you!
Hello: This patch was applied to chrome-platform/linux.git (for-next) by Prashant Malani <pmalani@chromium.org>: On Thu, 17 Nov 2022 08:08:23 +0000 you wrote: > The following WARNING message was given when rmmod cros_usbpd_notify: > > Unexpected driver unregister! > WARNING: CPU: 0 PID: 253 at drivers/base/driver.c:270 driver_unregister+0x8a/0xb0 > Modules linked in: cros_usbpd_notify(-) > CPU: 0 PID: 253 Comm: rmmod Not tainted 6.1.0-rc3 #24 > ... > Call Trace: > <TASK> > cros_usbpd_notify_exit+0x11/0x1e [cros_usbpd_notify] > __x64_sys_delete_module+0x3c7/0x570 > ? __ia32_sys_delete_module+0x570/0x570 > ? lock_is_held_type+0xe3/0x140 > ? syscall_enter_from_user_mode+0x17/0x50 > ? rcu_read_lock_sched_held+0xa0/0xd0 > ? syscall_enter_from_user_mode+0x1c/0x50 > do_syscall_64+0x37/0x90 > entry_SYSCALL_64_after_hwframe+0x63/0xcd > RIP: 0033:0x7f333fe9b1b7 > > [...] Here is the summary with links: - platform/chrome: cros_usbpd_notify: Fix error handling in cros_usbpd_notify_init() https://git.kernel.org/chrome-platform/c/5a2d96623670 You are awesome, thank you!
diff --git a/drivers/platform/chrome/cros_usbpd_notify.c b/drivers/platform/chrome/cros_usbpd_notify.c index 4b5a81c9dc6d..10670b6588e3 100644 --- a/drivers/platform/chrome/cros_usbpd_notify.c +++ b/drivers/platform/chrome/cros_usbpd_notify.c @@ -239,7 +239,11 @@ static int __init cros_usbpd_notify_init(void) return ret; #ifdef CONFIG_ACPI - platform_driver_register(&cros_usbpd_notify_acpi_driver); + ret = platform_driver_register(&cros_usbpd_notify_acpi_driver); + if (ret) { + platform_driver_unregister(&cros_usbpd_notify_plat_driver); + return ret; + } #endif return 0; }
The following WARNING message was given when rmmod cros_usbpd_notify: Unexpected driver unregister! WARNING: CPU: 0 PID: 253 at drivers/base/driver.c:270 driver_unregister+0x8a/0xb0 Modules linked in: cros_usbpd_notify(-) CPU: 0 PID: 253 Comm: rmmod Not tainted 6.1.0-rc3 #24 ... Call Trace: <TASK> cros_usbpd_notify_exit+0x11/0x1e [cros_usbpd_notify] __x64_sys_delete_module+0x3c7/0x570 ? __ia32_sys_delete_module+0x570/0x570 ? lock_is_held_type+0xe3/0x140 ? syscall_enter_from_user_mode+0x17/0x50 ? rcu_read_lock_sched_held+0xa0/0xd0 ? syscall_enter_from_user_mode+0x1c/0x50 do_syscall_64+0x37/0x90 entry_SYSCALL_64_after_hwframe+0x63/0xcd RIP: 0033:0x7f333fe9b1b7 The reason is that the cros_usbpd_notify_init() does not check the return value of platform_driver_register(), and the cros_usbpd_notify can install successfully even if platform_driver_register() failed. Fix by checking the return value of platform_driver_register() and unregister cros_usbpd_notify_plat_driver when it failed. Fixes: ec2daf6e33f9 ("platform: chrome: Add cros-usbpd-notify driver") Signed-off-by: Yuan Can <yuancan@huawei.com> --- drivers/platform/chrome/cros_usbpd_notify.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)