* Re: [PATCH] platform/chrome: cros_usbpd_notify: Fix error handling in cros_usbpd_notify_init()
2022-11-17 8:08 [PATCH] platform/chrome: cros_usbpd_notify: Fix error handling in cros_usbpd_notify_init() Yuan Can
@ 2022-11-17 19:07 ` Prashant Malani
2022-11-18 10:18 ` Yuan Can
2022-11-17 19:15 ` Brian Norris
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Prashant Malani @ 2022-11-17 19:07 UTC (permalink / raw)
To: Yuan Can; +Cc: bleung, gwendal, jflat, chrome-platform
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] platform/chrome: cros_usbpd_notify: Fix error handling in cros_usbpd_notify_init()
2022-11-17 19:07 ` Prashant Malani
@ 2022-11-18 10:18 ` Yuan Can
2022-11-18 20:11 ` Prashant Malani
0 siblings, 1 reply; 9+ messages in thread
From: Yuan Can @ 2022-11-18 10:18 UTC (permalink / raw)
To: Prashant Malani; +Cc: bleung, gwendal, jflat, chrome-platform
在 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
--
Best regards,
Yuan Can
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] platform/chrome: cros_usbpd_notify: Fix error handling in cros_usbpd_notify_init()
2022-11-18 10:18 ` Yuan Can
@ 2022-11-18 20:11 ` Prashant Malani
2022-11-23 8:33 ` Yuan Can
0 siblings, 1 reply; 9+ messages in thread
From: Prashant Malani @ 2022-11-18 20:11 UTC (permalink / raw)
To: Yuan Can; +Cc: bleung, gwendal, jflat, chrome-platform
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,
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] platform/chrome: cros_usbpd_notify: Fix error handling in cros_usbpd_notify_init()
2022-11-18 20:11 ` Prashant Malani
@ 2022-11-23 8:33 ` Yuan Can
2022-11-25 8:27 ` Prashant Malani
0 siblings, 1 reply; 9+ messages in thread
From: Yuan Can @ 2022-11-23 8:33 UTC (permalink / raw)
To: Prashant Malani; +Cc: bleung, gwendal, jflat, chrome-platform
在 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.
--
Best regards,
Yuan Can
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] platform/chrome: cros_usbpd_notify: Fix error handling in cros_usbpd_notify_init()
2022-11-23 8:33 ` Yuan Can
@ 2022-11-25 8:27 ` Prashant Malani
0 siblings, 0 replies; 9+ messages in thread
From: Prashant Malani @ 2022-11-25 8:27 UTC (permalink / raw)
To: Yuan Can; +Cc: bleung, gwendal, jflat, chrome-platform
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] platform/chrome: cros_usbpd_notify: Fix error handling in cros_usbpd_notify_init()
2022-11-17 8:08 [PATCH] platform/chrome: cros_usbpd_notify: Fix error handling in cros_usbpd_notify_init() Yuan Can
2022-11-17 19:07 ` Prashant Malani
@ 2022-11-17 19:15 ` Brian Norris
2022-11-25 8:40 ` patchwork-bot+chrome-platform
2022-11-28 20:00 ` patchwork-bot+chrome-platform
3 siblings, 0 replies; 9+ messages in thread
From: Brian Norris @ 2022-11-17 19:15 UTC (permalink / raw)
To: Yuan Can; +Cc: pmalani, bleung, gwendal, jflat, chrome-platform
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
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] platform/chrome: cros_usbpd_notify: Fix error handling in cros_usbpd_notify_init()
2022-11-17 8:08 [PATCH] platform/chrome: cros_usbpd_notify: Fix error handling in cros_usbpd_notify_init() Yuan Can
2022-11-17 19:07 ` Prashant Malani
2022-11-17 19:15 ` Brian Norris
@ 2022-11-25 8:40 ` patchwork-bot+chrome-platform
2022-11-28 20:00 ` patchwork-bot+chrome-platform
3 siblings, 0 replies; 9+ messages in thread
From: patchwork-bot+chrome-platform @ 2022-11-25 8:40 UTC (permalink / raw)
To: Yuan Can; +Cc: pmalani, bleung, gwendal, jflat, chrome-platform
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!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] platform/chrome: cros_usbpd_notify: Fix error handling in cros_usbpd_notify_init()
2022-11-17 8:08 [PATCH] platform/chrome: cros_usbpd_notify: Fix error handling in cros_usbpd_notify_init() Yuan Can
` (2 preceding siblings ...)
2022-11-25 8:40 ` patchwork-bot+chrome-platform
@ 2022-11-28 20:00 ` patchwork-bot+chrome-platform
3 siblings, 0 replies; 9+ messages in thread
From: patchwork-bot+chrome-platform @ 2022-11-28 20:00 UTC (permalink / raw)
To: Yuan Can; +Cc: pmalani, bleung, gwendal, jflat, chrome-platform
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!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 9+ messages in thread