2019-03-19 16:33:41

by Guenter Roeck

[permalink] [raw]
Subject: [PATCH] platform/chrome: cros_ec_debugfs: cancel/schedule logging work only if supported

The following traceback was reported on ASUS C201, which does not support
console logging.

------------[ cut here ]------------
WARNING: CPU: 2 PID: 361 at kernel/workqueue.c:3030 __flush_work+0x38/0x154
Modules linked in: snd_soc_hdmi_codec cros_ec_debugfs cros_ec_sysfs uvcvideo dw_hdmi_cec dw_hdmi_i2s_audio videobuf2_vmalloc cfg80211 gpio_charger rk_crypto rfkill videobuf2_memops videobuf2_v4l2 des_generic videobuf2_common ofpart m25p80 spi_nor tpm_i2c_infineon sbs_battery mtd tpm joydev cros_ec_dev coreboot_table evdev mousedev ip_tables x_tables [last unloaded: brcmutil]
CPU: 2 PID: 361 Comm: systemd-sleep Not tainted 5.1.0-rc1-1-ARCH+ #1
Hardware name: Rockchip (Device Tree)
[<c020e4b0>] (unwind_backtrace) from [<c020ac18>] (show_stack+0x10/0x14)
[<c020ac18>] (show_stack) from [<c07a3e04>] (dump_stack+0x7c/0x9c)
[<c07a3e04>] (dump_stack) from [<c0222748>] (__warn+0xd0/0xec)
[<c0222748>] (__warn) from [<c022279c>] (warn_slowpath_null+0x38/0x44)
[<c022279c>] (warn_slowpath_null) from [<c02365d0>] (__flush_work+0x38/0x154)
[<c02365d0>] (__flush_work) from [<c023786c>] (__cancel_work_timer+0x114/0x1a4)
[<c023786c>] (__cancel_work_timer) from [<bf33233c>] (cros_ec_debugfs_suspend+0x14/0x1c [cros_ec_debugfs])
[<bf33233c>] (cros_ec_debugfs_suspend [cros_ec_debugfs]) from [<c056a888>] (dpm_run_callback+0x64/0xcc)
[<c056a888>] (dpm_run_callback) from [<c056ad2c>] (__device_suspend+0x174/0x3a8)
[<c056ad2c>] (__device_suspend) from [<c056b9e0>] (dpm_suspend+0x174/0x1e0)
[<c056b9e0>] (dpm_suspend) from [<c026b3e0>] (suspend_devices_and_enter+0x6c/0x50c)
[<c026b3e0>] (suspend_devices_and_enter) from [<c026ba8c>] (pm_suspend+0x20c/0x274)
[<c026ba8c>] (pm_suspend) from [<c026a628>] (state_store+0x54/0x88)
[<c026a628>] (state_store) from [<c03cd2d0>] (kernfs_fop_write+0x114/0x180)
[<c03cd2d0>] (kernfs_fop_write) from [<c035d48c>] (__vfs_write+0x1c/0x154)
[<c035d48c>] (__vfs_write) from [<c035f9e8>] (vfs_write+0xb8/0x198)
[<c035f9e8>] (vfs_write) from [<c035fbc0>] (ksys_write+0x3c/0x74)
[<c035fbc0>] (ksys_write) from [<c0201000>] (ret_fast_syscall+0x0/0x4c)
Exception stack(0xe9365fa8 to 0xe9365ff0)
5fa0: 00000004 beef8b28 00000004 beef8b28 00000004 00000000
5fc0: 00000004 beef8b28 02319170 00000004 beef8b28 00000004 b6f3d900 beef8b74
5fe0: 0000006c beef8a98 b6c0adac b6c66534
---[ end trace f4ee5df14e8ea0ec ]---

If console logging is not supported, the work structure is never
initialized, resulting in the traceback. Calling cancel/schedule functions
conditionally fixes the problem.

While at it, also fix error handling in the probe function.

Reported-by: Urja Rannikko <[email protected]>
Cc: Urja Rannikko <[email protected]>
Fixes: 6fce0a2cf5a05 ("mfd / platform: cros_ec: Move debugfs attributes to its own driver")
Signed-off-by: Guenter Roeck <[email protected]>
---
drivers/platform/chrome/cros_ec_debugfs.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_debugfs.c b/drivers/platform/chrome/cros_ec_debugfs.c
index 900c7073c46f..71308766e891 100644
--- a/drivers/platform/chrome/cros_ec_debugfs.c
+++ b/drivers/platform/chrome/cros_ec_debugfs.c
@@ -440,7 +440,7 @@ static int cros_ec_debugfs_probe(struct platform_device *pd)

ret = cros_ec_create_pdinfo(debug_info);
if (ret)
- goto remove_debugfs;
+ goto remove_log;

ec->debug_info = debug_info;

@@ -448,6 +448,8 @@ static int cros_ec_debugfs_probe(struct platform_device *pd)

return 0;

+remove_log:
+ cros_ec_cleanup_console_log(debug_info);
remove_debugfs:
debugfs_remove_recursive(debug_info->dir);
return ret;
@@ -467,7 +469,8 @@ static int __maybe_unused cros_ec_debugfs_suspend(struct device *dev)
{
struct cros_ec_dev *ec = dev_get_drvdata(dev);

- cancel_delayed_work_sync(&ec->debug_info->log_poll_work);
+ if (ec->debug_info->log_buffer.buf)
+ cancel_delayed_work_sync(&ec->debug_info->log_poll_work);

return 0;
}
@@ -476,7 +479,8 @@ static int __maybe_unused cros_ec_debugfs_resume(struct device *dev)
{
struct cros_ec_dev *ec = dev_get_drvdata(dev);

- schedule_delayed_work(&ec->debug_info->log_poll_work, 0);
+ if (ec->debug_info->log_buffer.buf)
+ schedule_delayed_work(&ec->debug_info->log_poll_work, 0);

return 0;
}
--
2.7.4



2019-03-19 18:03:46

by Benson Leung

[permalink] [raw]
Subject: Re: [PATCH] platform/chrome: cros_ec_debugfs: cancel/schedule logging work only if supported

Hi Guenter,

On Tue, Mar 19, 2019 at 09:32:36AM -0700, Guenter Roeck wrote:
> The following traceback was reported on ASUS C201, which does not support
> console logging.
>
> ------------[ cut here ]------------
> WARNING: CPU: 2 PID: 361 at kernel/workqueue.c:3030 __flush_work+0x38/0x154
> Modules linked in: snd_soc_hdmi_codec cros_ec_debugfs cros_ec_sysfs uvcvideo dw_hdmi_cec dw_hdmi_i2s_audio videobuf2_vmalloc cfg80211 gpio_charger rk_crypto rfkill videobuf2_memops videobuf2_v4l2 des_generic videobuf2_common ofpart m25p80 spi_nor tpm_i2c_infineon sbs_battery mtd tpm joydev cros_ec_dev coreboot_table evdev mousedev ip_tables x_tables [last unloaded: brcmutil]
> CPU: 2 PID: 361 Comm: systemd-sleep Not tainted 5.1.0-rc1-1-ARCH+ #1
> Hardware name: Rockchip (Device Tree)
> [<c020e4b0>] (unwind_backtrace) from [<c020ac18>] (show_stack+0x10/0x14)
> [<c020ac18>] (show_stack) from [<c07a3e04>] (dump_stack+0x7c/0x9c)
> [<c07a3e04>] (dump_stack) from [<c0222748>] (__warn+0xd0/0xec)
> [<c0222748>] (__warn) from [<c022279c>] (warn_slowpath_null+0x38/0x44)
> [<c022279c>] (warn_slowpath_null) from [<c02365d0>] (__flush_work+0x38/0x154)
> [<c02365d0>] (__flush_work) from [<c023786c>] (__cancel_work_timer+0x114/0x1a4)
> [<c023786c>] (__cancel_work_timer) from [<bf33233c>] (cros_ec_debugfs_suspend+0x14/0x1c [cros_ec_debugfs])
> [<bf33233c>] (cros_ec_debugfs_suspend [cros_ec_debugfs]) from [<c056a888>] (dpm_run_callback+0x64/0xcc)
> [<c056a888>] (dpm_run_callback) from [<c056ad2c>] (__device_suspend+0x174/0x3a8)
> [<c056ad2c>] (__device_suspend) from [<c056b9e0>] (dpm_suspend+0x174/0x1e0)
> [<c056b9e0>] (dpm_suspend) from [<c026b3e0>] (suspend_devices_and_enter+0x6c/0x50c)
> [<c026b3e0>] (suspend_devices_and_enter) from [<c026ba8c>] (pm_suspend+0x20c/0x274)
> [<c026ba8c>] (pm_suspend) from [<c026a628>] (state_store+0x54/0x88)
> [<c026a628>] (state_store) from [<c03cd2d0>] (kernfs_fop_write+0x114/0x180)
> [<c03cd2d0>] (kernfs_fop_write) from [<c035d48c>] (__vfs_write+0x1c/0x154)
> [<c035d48c>] (__vfs_write) from [<c035f9e8>] (vfs_write+0xb8/0x198)
> [<c035f9e8>] (vfs_write) from [<c035fbc0>] (ksys_write+0x3c/0x74)
> [<c035fbc0>] (ksys_write) from [<c0201000>] (ret_fast_syscall+0x0/0x4c)
> Exception stack(0xe9365fa8 to 0xe9365ff0)
> 5fa0: 00000004 beef8b28 00000004 beef8b28 00000004 00000000
> 5fc0: 00000004 beef8b28 02319170 00000004 beef8b28 00000004 b6f3d900 beef8b74
> 5fe0: 0000006c beef8a98 b6c0adac b6c66534
> ---[ end trace f4ee5df14e8ea0ec ]---
>
> If console logging is not supported, the work structure is never
> initialized, resulting in the traceback. Calling cancel/schedule functions
> conditionally fixes the problem.
>
> While at it, also fix error handling in the probe function.
>
> Reported-by: Urja Rannikko <[email protected]>
> Cc: Urja Rannikko <[email protected]>
> Fixes: 6fce0a2cf5a05 ("mfd / platform: cros_ec: Move debugfs attributes to its own driver")
> Signed-off-by: Guenter Roeck <[email protected]>


Looks good. Applied to 5.1-fixes.

Thanks for the quick fix.
Benson


--
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
[email protected]
Chromium OS Project
[email protected]


Attachments:
(No filename) (3.15 kB)
signature.asc (849.00 B)
Download all attachments