linux-kernel - Re: [PATCH v1] thermal/debugfs: Remove unnecessary debugfs_create_dir() error check in thermal_debug

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Mon, 15 Jan 2024 18:09:06 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Daniel Lezcano <daniel.lezcano@...aro.org>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, Minjie Du <duminjie@...o.com>, 
	Zhang Rui <rui.zhang@...el.com>, Lukasz Luba <lukasz.luba@....com>, 
	"open list:THERMAL" <linux-pm@...r.kernel.org>, open list <linux-kernel@...r.kernel.org>, 
	opensource.kernel@...o.com
Subject: Re: [PATCH v1] thermal/debugfs: Remove unnecessary
 debugfs_create_dir() error check in thermal_debug_init()

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