Re: [PATCH v1] thermal/debugfs: Remove unnecessary debugfs_create_dir() error check in thermal_debug_init()

[Rafael J. Wysocki]

On Mon, Jan 15, 2024 at 5:55 PM Daniel Lezcano
<daniel.lezcano@xxxxxxxxxx> wrote:
>
> On 15/01/2024 16:52, Rafael J. Wysocki wrote:
> > On Mon, Jan 15, 2024 at 9:25 AM Minjie Du <duminjie@xxxxxxxx> 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.