linux-kernel - Re: [PATCH v2 3/3] hwmon: (ltc2992) Use fwnode_for_each_available_child_node

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] [thread-next>] [day] [month] [year] [list]

Date: Wed, 26 Jun 2024 08:33:35 +0200
From: Nuno Sá <noname.nuno@...il.com>
To: Andy Shevchenko <andy.shevchenko@...il.com>, Jonathan Cameron
	 <jic23@...nel.org>
Cc: Javier Carrasco <javier.carrasco.cruz@...il.com>, Greg Kroah-Hartman
 <gregkh@...uxfoundation.org>, "Rafael J. Wysocki" <rafael@...nel.org>, Andy
 Shevchenko <andriy.shevchenko@...ux.intel.com>, Daniel Scally
 <djrscally@...il.com>, Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
 Sakari Ailus <sakari.ailus@...ux.intel.com>,  Jean Delvare
 <jdelvare@...e.com>, Guenter Roeck <linux@...ck-us.net>, Antoniu Miclaus
 <antoniu.miclaus@...log.com>, linux-acpi@...r.kernel.org, 
 linux-kernel@...r.kernel.org, linux-hwmon@...r.kernel.org, Rob Herring
 <robh@...nel.org>, devicetree@...r.kernel.org
Subject: Re: [PATCH v2 3/3] hwmon: (ltc2992) Use
 fwnode_for_each_available_child_node_scoped()

On Mon, 2024-05-27 at 17:30 +0300, Andy Shevchenko wrote:
> Sun, May 26, 2024 at 02:48:51PM +0100, Jonathan Cameron kirjoitti:
> > On Thu, 23 May 2024 17:47:16 +0200
> > Javier Carrasco <javier.carrasco.cruz@...il.com> wrote:
> > 
> > > The scoped version of the fwnode_for_each_available_child_node() macro
> > > automates object recfount decrement, avoiding possible memory leaks
> > > in new error paths inside the loop like it happened when
> > > commit '10b029020487 ("hwmon: (ltc2992) Avoid division by zero")'
> > > was added.
> > > 
> > > The new macro removes the need to manually call fwnode_handle_put() in
> > > the existing error paths and in any future addition. It also removes the
> > > need for the current child node declaration as well, as it is internally
> > > declared.
> > > 
> > > Reviewed-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> > > Signed-off-by: Javier Carrasco <javier.carrasco.cruz@...il.com>
> > 
> > This looks like another instances of the lack of clarify about 
> > what device_for_each_child_node[_scoped]() guarantees about node
> > availability.
> > On DT it guarantees the node is available as ultimately calls
> > of_get_next_available_child()
> > 
> > On ACPI it doesn't (I think).
> > For swnode, there isn't an obvious concept of available.
> > 
> > It would be much better if we reached some agreement on this and
> > hence could avoid using the fwnode variants just to get the _available_ form
> > as done here.
> 
> > Or just add the device_for_each_available_child_node[_scoped]()
> > and call that in almost all cases.
> 
> device_for_each*() _implies_ availability. You need to talk to Rob about all
> this. The design of the device_for_each*() was exactly done in accordance with
> his suggestions...
> 
> > In generic code, do we ever want to walk unavailable child nodes?
> 
> ...which are most likely like your question here, i.e. why we ever need to
> traverse over unavailable nodes.
> 

I have some vague idea of Rob talking about CPUs being one of the reasons for
the current design. Don't remember for sure. At least (if not already) having
this clearly documented would be nice.

- Nuno Sá