Re: [PATCH] driver core: Fix userspace expectations of uevent_show() as a probe barrier
From: Dan Williams
Date: Wed Oct 02 2024 - 17:51:22 EST
Greg KH wrote:
> On Mon, Sep 30, 2024 at 01:05:13PM -0700, Dan Williams wrote:
> > Commit "driver core: Fix uevent_show() vs driver detach race" [1]
> > attempted to fix a lockdep report in uevent_show() by making the lookup
> > of driver information for a given device lockless. It turns out that
> > userspace likely depends on the side-effect of uevent_show() flushing
> > device probing, as evidenced by the removal of the lock causing a
> > regression initializing USB devices [2].
> >
> > Introduce a new "locked" type for sysfs attributes that arranges for the
> > attribute to be called under the device-lock, but without setting up a
> > reverse locking order dependency with the kernfs_get_active() lock.
>
> As this is "only" for the driver core, can you move some of the stuff in
> the global .h files to the local ones so that no one else gets the wrong
> idea they can use them?
Makes sense. Moving SYSFS_LOCKED and related bits to fs/sysfs/sysfs.h
is straightforward. It did result in the below hunk inside
drivers/base/base.h, but let me know on the v2 posting if that's
irksome:
diff --git a/drivers/base/base.h b/drivers/base/base.h
index 0b53593372d7..0405e7667184 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -246,3 +246,12 @@ static inline int devtmpfs_delete_node(struct device *dev) { return 0; }
void software_node_notify(struct device *dev);
void software_node_notify_remove(struct device *dev);
+
+/*
+ * "locked" device attributes are an internal exception for the @uevent
+ * attribute.
+ */
+#include "../../fs/sysfs/sysfs.h"
+
+#define DEVICE_ATTR_LOCKED_RW(_name) \
+ struct device_attribute dev_attr_##_name = __ATTR_LOCKED_RW(_name)