Messages in this thread | | | From | "Rafael J. Wysocki" <> | Date | Wed, 22 Jun 2022 15:55:00 +0200 | Subject | Re: [PATCH 1/1] thermal/sysfs: Clear the slot left in cooling_device_attr_groups |
| |
On Mon, Jun 20, 2022 at 3:20 PM Di Shen <di.shen@unisoc.com> wrote: > > There's a space allocated for cooling_device_stats_attr_group within co-
Please don't break words in the changelog (and variable names in particular) like this.
> oling_device_attr_groups. This space is shared by all cooling devices. > > If not clear this space before cooling_device_stats_attr_group is initi- > alized, the next cooling device will still create stats sysfs file node. > At this time, read or write related nodes will cause kernel crash.
A bit more of an explanation here wouldn't hurt IMV. In particular, what does "the next cooling device" mean and what are "related nodes"?
> [exception_reboot_reason]: kernel_crash > [exception_panic_reason]: Fatal exception > [exception_time]: 2022-06-07_06-32-25 > [exception_file_info]: not-bugon > [exception_task_id]: 3154 > [exception_task_family]: [cat, 3154][sh, 2597][sh, 2362][adbd, 1804] > [exception_pc_symbol]: [<747516ae>] _raw_spin_lock+0x20/0x60 > [exception_stack_info]: [<4cbe9ac1>] prepare_exception_info+0x19c/0x1a4 > [<03041be7>] sysdump_panic_event+0x57c/0x6f4 > [<b16f865e>] atomic_notifier_call_chain+0x48/0x7c > [<5baac8d4>] panic+0x1b4/0x3c8 > [<9d287b0f>] arm_notify_die+0x0/0x78 > [<094fc22c>] __do_kernel_fault+0x94/0xa4 > [<3b4b69a4>] do_page_fault+0xd4/0x364 > [<23793e7a>] do_translation_fault+0x38/0xc0 > [<6e5cc52a>] do_DataAbort+0x4c/0xd0 > [<a28c16b8>] __dabt_svc+0x5c/0xa0 > [<747516ae>] _raw_spin_lock+0x20/0x60 > [<9a9e4cd4>] time_in_state_ms_show+0x28/0x148 > [<cb78325e>] dev_attr_show+0x38/0x64 > [<aea3e364>] sysfs_kf_seq_show+0x8c/0xf0 > [<c0a843ab>] seq_read+0x244/0x620 > [<b316b374>] vfs_read+0xd8/0x218 > [<3aebf5fa>] sys_read+0x80/0xe4 > [<7cf100f5>] ret_fast_syscall+0x0/0x28 > [<08cbe22f>] 0xbe8c1198 > > So clear the slot left in cooling_device_attr_groups before cooling_dev- > ice_stats_attr_group is initialized to avoid kernel crash. > > Signed-off-by: Di Shen <di.shen@unisoc.com> > --- > drivers/thermal/thermal_sysfs.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c > index 1c4aac8464a7..fbc3dbc85841 100644 > --- a/drivers/thermal/thermal_sysfs.c > +++ b/drivers/thermal/thermal_sysfs.c > @@ -817,6 +817,11 @@ static void cooling_device_stats_setup(struct thermal_cooling_device *cdev) > unsigned long states; > int var; > > + /* Clear the slot left in cooling_device_attr_groups */
The comment is a bit too vague IMO. In particular, what slot do you mean?
> + var = ARRAY_SIZE(cooling_device_attr_groups) - 2; > + if (cooling_device_attr_groups[var]) > + cooling_device_attr_groups[var] = NULL;
The NULL check above is redundant (it can be cleared even if it is NULL already) and it all can be done in one code line.
> + > if (cdev->ops->get_max_state(cdev, &states)) > return; > > --
| |