Received-SPF: pass (google.com: domain of linux-kernel+bounces-26296-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <20240115082507.29651-1-duminjie@vivo.com> <CAJZ5v0jPeYgGc9xmSrnLsg6RkhzmU=TfPdhrzxWBd_d_dmJh+Q@mail.gmail.com>
 <ecb33c61-bb40-43a0-94a1-8973d70dc0b4@linaro.org>
In-Reply-To: <ecb33c61-bb40-43a0-94a1-8973d70dc0b4@linaro.org>
From: "Rafael J. Wysocki" <rafael@kernel.org>
Date: Mon, 15 Jan 2024 18:09:06 +0100
Message-ID: <CAJZ5v0hhvwz++RUtw-e4EM10Rs0ipOUQBXi4GUH0oL01iUJKrQ@mail.gmail.com>
Subject: Re: [PATCH v1] thermal/debugfs: Remove unnecessary
 debugfs_create_dir() error check in thermal_debug_init()
To: Daniel Lezcano <daniel.lezcano@linaro.org>
Cc: "Rafael J. Wysocki" <rafael@kernel.org>, Minjie Du <duminjie@vivo.com>, 
	Zhang Rui <rui.zhang@intel.com>, Lukasz Luba <lukasz.luba@arm.com>, 
	"open list:THERMAL" <linux-pm@vger.kernel.org>, open list <linux-kernel@vger.kernel.org>, 
	opensource.kernel@vivo.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Mon, Jan 15, 2024 at 5:55=E2=80=AFPM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
> On 15/01/2024 16:52, Rafael J. Wysocki wrote:
> > On Mon, Jan 15, 2024 at 9:25=E2=80=AFAM Minjie Du <duminjie@vivo.com> w=
rote:
> >>
> >> 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 =3D debugfs_create_dir("thermal", NULL);
> d_cdev =3D debugfs_create_dir("cooling_devices", d_root);
> d_tz =3D 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.