This patch removes the debugfs_create_dir() error checking in
thermal_debug_init(). Because the debugfs_create_dir() is developed
in a way that the caller can safely handle the errors that
occur during the creation of DebugFS nodes.
Signed-off-by: Minjie Du <[email protected]>
---
drivers/thermal/thermal_debugfs.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/thermal/thermal_debugfs.c b/drivers/thermal/thermal_debugfs.c
index a3fa09235da1..695253559a61 100644
--- a/drivers/thermal/thermal_debugfs.c
+++ b/drivers/thermal/thermal_debugfs.c
@@ -172,12 +172,8 @@ struct thermal_debugfs {
void thermal_debug_init(void)
{
d_root = debugfs_create_dir("thermal", NULL);
- if (!d_root)
- return;
d_cdev = debugfs_create_dir("cooling_devices", d_root);
- if (!d_cdev)
- return;
d_tz = debugfs_create_dir("thermal_zones", d_root);
}
--
2.39.0
On Mon, Jan 15, 2024 at 9:25 AM Minjie Du <[email protected]> wrote:
>
> This patch removes the debugfs_create_dir() error checking in
> thermal_debug_init(). Because the debugfs_create_dir() is developed
> in a way that the caller can safely handle the errors that
> occur during the creation of DebugFS nodes.
I honestly don't see what the purpose of this patch is.
> Signed-off-by: Minjie Du <[email protected]>
> ---
> drivers/thermal/thermal_debugfs.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/drivers/thermal/thermal_debugfs.c b/drivers/thermal/thermal_debugfs.c
> index a3fa09235da1..695253559a61 100644
> --- a/drivers/thermal/thermal_debugfs.c
> +++ b/drivers/thermal/thermal_debugfs.c
> @@ -172,12 +172,8 @@ struct thermal_debugfs {
> void thermal_debug_init(void)
> {
> d_root = debugfs_create_dir("thermal", NULL);
> - if (!d_root)
> - return;
>
> d_cdev = debugfs_create_dir("cooling_devices", d_root);
> - if (!d_cdev)
> - return;
>
> d_tz = debugfs_create_dir("thermal_zones", d_root);
> }
> --
On 15/01/2024 16:52, Rafael J. Wysocki wrote:
> On Mon, Jan 15, 2024 at 9:25 AM Minjie Du <[email protected]> wrote:
>>
>> This patch removes the debugfs_create_dir() error checking in
>> thermal_debug_init(). Because the debugfs_create_dir() is developed
>> in a way that the caller can safely handle the errors that
>> occur during the creation of DebugFS nodes.
>
> I honestly don't see what the purpose of this patch is.
I think it is because the recent debugfs changes were about to reduce as
much as possible the code related to the error handling as the debugfs
is not supposed to go in production system.
So for instance debugfs_create_dir() will not fail if the parent is NULL
and will create the entry in the debugfs topdir.
At the end we are ending up with:
d_root = debugfs_create_dir("thermal", NULL);
d_cdev = debugfs_create_dir("cooling_devices", d_root);
d_tz = debugfs_create_dir("thermal_zones", d_root);
The current code will avoid creating lost entries in /sys/kernel/debug
The proposed change will create those in /sys/kernel/debug in case of error.
Note I reduced as much as possible the error handling in this function.
So it is a matter to reduce it even more but may be resulting in junk in
/sys/kernel/debug in case of error
TBH, I'm 50/50 :)
>> Signed-off-by: Minjie Du <[email protected]>
>> ---
>> drivers/thermal/thermal_debugfs.c | 4 ----
>> 1 file changed, 4 deletions(-)
>>
>> diff --git a/drivers/thermal/thermal_debugfs.c b/drivers/thermal/thermal_debugfs.c
>> index a3fa09235da1..695253559a61 100644
>> --- a/drivers/thermal/thermal_debugfs.c
>> +++ b/drivers/thermal/thermal_debugfs.c
>> @@ -172,12 +172,8 @@ struct thermal_debugfs {
>> void thermal_debug_init(void)
>> {
>> d_root = debugfs_create_dir("thermal", NULL);
>> - if (!d_root)
>> - return;
>>
>> d_cdev = debugfs_create_dir("cooling_devices", d_root);
>> - if (!d_cdev)
>> - return;
>>
>> d_tz = debugfs_create_dir("thermal_zones", d_root);
>> }
>> --
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
On Mon, Jan 15, 2024 at 5:55 PM Daniel Lezcano
<[email protected]> wrote:
>
> On 15/01/2024 16:52, Rafael J. Wysocki wrote:
> > On Mon, Jan 15, 2024 at 9:25 AM Minjie Du <[email protected]> wrote:
> >>
> >> This patch removes the debugfs_create_dir() error checking in
> >> thermal_debug_init(). Because the debugfs_create_dir() is developed
> >> in a way that the caller can safely handle the errors that
> >> occur during the creation of DebugFS nodes.
> >
> > I honestly don't see what the purpose of this patch is.
>
> I think it is because the recent debugfs changes were about to reduce as
> much as possible the code related to the error handling as the debugfs
> is not supposed to go in production system.
>
> So for instance debugfs_create_dir() will not fail if the parent is NULL
> and will create the entry in the debugfs topdir.
Which would be confusing IMO.
> At the end we are ending up with:
>
> d_root = debugfs_create_dir("thermal", NULL);
> d_cdev = debugfs_create_dir("cooling_devices", d_root);
> d_tz = debugfs_create_dir("thermal_zones", d_root);
>
> The current code will avoid creating lost entries in /sys/kernel/debug
Right, and IMO it is the right thing to do, and I would even go a bit
further and roll back the thermal debugfs initialization if any of the
above is NULL.
Note that currently d_tz can be NULL and it will work in the sort of
degraded mode with the other two, but it is not a big deal IMV. It
would just be more consistent to clean up everything then, but that
can be done later.