Thread (16 messages) 16 messages, 7 authors, 2026-03-18

Re: [PATCH] device property: Make modifications of fwnode "flags" thread safe

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: 2026-03-17 07:10:37
Also in: driver-core, imx, linux-acpi, linux-arm-kernel, linux-devicetree, linux-i2c, linux-spi, lkml, stable

On Mon, Mar 16, 2026 at 03:42:06PM -0700, Douglas Anderson wrote:
In various places in the kernel, we modify the fwnode "flags" member
by doing either:
  fwnode->flags |= SOME_FLAG;
  fwnode->flags &= ~SOME_FLAG;

This type of modification is not thread-safe. If two threads are both
mucking with the flags at the same time then one can clobber the
other.

While flags are often modified while under the "fwnode_link_lock",
this is not universally true.

Create some accessor functions for setting, clearing, and testing the
FWNODE flags and move all users to these accessor functions. New
accessor functions use set_bit() and clear_bit(), which are
thread-safe.
In general I like the idea (independently on the treewide patch or a series).
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

See one nit-pick below, though.

...
While this patch is not known for sure to fix any specific issues, it
seems possible that it could fix some rare problems. I'm currently
trying to track down a hard-to-reproduce heisenbug and one (currently
unproven) theory I had was that the fwnode flags could be getting
messed up like this. Even if turns out not to fix my heisenbug,
though, this seems like a worthwhile change to take.
Totally agree. This makes code more OOP like and robust against subtle
modifications.

...
+static inline void fwnode_set_flag(struct fwnode_handle *fwnode,
+				   unsigned int bit)
+{
+	set_bit(bit, &fwnode->flags);
+}
+
+static inline void fwnode_clear_flag(struct fwnode_handle *fwnode,
+				   unsigned int bit)
+{
+	clear_bit(bit, &fwnode->flags);
+}
Perhaps you also want to have fwnode_assign_flag() (assign_bit() underneath)
as...
+static inline bool fwnode_test_flag(struct fwnode_handle *fwnode,
+				    unsigned int bit)
+{
+	return test_bit(bit, &fwnode->flags);
+}
 	if (initialized)
-		fwnode->flags |= FWNODE_FLAG_INITIALIZED;
+		fwnode_set_flag(fwnode, FWNODE_FLAG_INITIALIZED);
 	else
-		fwnode->flags &= ~FWNODE_FLAG_INITIALIZED;
+		fwnode_clear_flag(fwnode, FWNODE_FLAG_INITIALIZED);
...at least one immediate user here.

-- 
With Best Regards,
Andy Shevchenko

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help