[RFC PATCH 02/14] sysfs: add existence-check helpers for lazy populate races

Pavol Sakac <[email protected]>
Newsgroups org.infradead.lists.kexec,dev.linux.lists.driver-core,org.kernel.vger.linux-kernel,org.kvack.linux-mm
Message-ID <[email protected]>
Lazy populate paths (introduced in subsequent commits) acquire a
per-device lock and may walk a table of create callbacks.
A populate_one("X") and populate_all() racing on the same device
serialize through that lock, but each create callback must verify
the entry doesn't already exist before calling sysfs_create_*().
Otherwise the second caller hits __kernfs_create_* which fires
sysfs_warn_dup() before the create callback can absorb -EEXIST.

Add two read-only helpers that wrap kernfs_find_and_get() so
create callbacks can perform the existence check under the same
lock that excludes other lazy create paths. Pairing exists() with
create() under lock makes the check-and-act atomic with respect
to all lazy populate paths on the device, and eager paths are
temporally separated (eager creates run during device_add, before
lazy is enabled).

  - sysfs_kn_exists(kobj, name): true iff @kobj has a child kernfs
    node named @name (any type).

  - sysfs_group_exists(kobj, grp):
    * grp->name != NULL: true iff a KERNFS_DIR child named
      grp->name exists under @kobj->sd (per-attribute presence is
      not inspected; mirrors how internal_create_group()
      short-circuits on -EEXIST).
    * grp->name == NULL: true iff every visible attribute in
      grp->attrs and grp->bin_attrs is present; a partially
      populated unnamed group returns false so a follow-up create
      pass can fill the missing entries.

Both take kernfs_rwsem(read) briefly via kernfs_find_and_get and
return false on bad input or non-existent name; sysfs_group_exists
additionally returns false on a node-type mismatch.

This commit only adds the helpers; subsequent commits convert
create callbacks to use them.

Cc: Greg Kroah-Hartman <[email protected]>
Cc: Rafael J. Wysocki <[email protected]>
Cc: Tejun Heo <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Danilo Krummrich <[email protected]>
Cc: [email protected]
Assisted-by: Claude:claude-opus-4.7
Signed-off-by: Pavol Sakac <[email protected]>
---
 fs/sysfs/dir.c        | 25 ++++++++++++++++
 fs/sysfs/file.c       |  2 ++
 fs/sysfs/group.c      | 70 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/sysfs.h | 15 ++++++++++
 4 files changed, 112 insertions(+)

diff --git a/fs/sysfs/dir.c b/fs/sysfs/dir.c
index ffdcd4153c584..ae97ab7e41939 100644
--- a/fs/sysfs/dir.c
+++ b/fs/sysfs/dir.c
@@ -159,3 +159,28 @@ void sysfs_remove_mount_point(struct kobject *parent_kobj, const char *name)
 	kernfs_remove_by_name_ns(parent, name, NULL);
 }
 EXPORT_SYMBOL_GPL(sysfs_remove_mount_point);
+
+
+/**
+ * sysfs_kn_exists - check whether any sysfs entry with @name exists
+ * @kobj: kobject under which to look
+ * @name: name to look up
+ *
+ * Existence probe under per-device sysfs_lazy_state.lock; lets a
+ * caller skip a redundant sysfs_create_*() and the resulting
+ * sysfs_warn_dup() WARN.
+ *
+ * Return: true if a kernfs node of any type with @name exists.
+ */
+bool sysfs_kn_exists(struct kobject *kobj, const char *name)
+{
+	struct kernfs_node *kn;
+
+	if (!kobj || !kobj->sd || !name)
+		return false;
+	kn = kernfs_find_and_get(kobj->sd, name);
+	if (!kn)
+		return false;
+	kernfs_put(kn);
+	return true;
+}
diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 5709cede1d756..5f3144e52ab72 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -816,3 +816,5 @@ ssize_t sysfs_bin_attr_simple_read(struct file *file, struct kobject *kobj,
 	return count;
 }
 EXPORT_SYMBOL_GPL(sysfs_bin_attr_simple_read);
+
+
diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c
index 182e54e575ee9..d6b2034cc22c9 100644
--- a/fs/sysfs/group.c
+++ b/fs/sysfs/group.c
@@ -636,3 +636,73 @@ int sysfs_groups_change_owner(struct kobject *kobj,
 	return error;
 }
 EXPORT_SYMBOL_GPL(sysfs_groups_change_owner);
+
+
+/**
+ * sysfs_group_exists - check whether an attribute_group is materialised
+ * @kobj: kobject under which to look
+ * @grp: attribute_group descriptor (may NOT be NULL)
+ *
+ * Read-only existence probe used by lazy populate paths.
+ *
+ * Two shapes:
+ *  - @grp->name != NULL: the group owns its own subdirectory.
+ *    Returns true iff a child kernfs_node of type KERNFS_DIR named
+ *    @grp->name exists under @kobj->sd. Per-attribute presence inside
+ *    the subdirectory is NOT inspected - once the subdir exists the
+ *    create path treats the named group as already materialised
+ *    (consistent with how internal_create_group() short-circuits its
+ *    create on -EEXIST).
+ *  - @grp->name == NULL: the group's attributes live directly under
+ *    @kobj->sd. Returns true iff every visible attribute in
+ *    @grp->attrs and @grp->bin_attrs is already present. A partially-
+ *    populated unnamed group returns false so a follow-up create
+ *    pass can fill the missing entries.
+ *
+ * Locking expectations: see sysfs_kn_exists().
+ *
+ * Return: true if the group is fully present per the rules above,
+ * false otherwise (including @grp == NULL).
+ */
+bool sysfs_group_exists(struct kobject *kobj, const struct attribute_group *grp)
+{
+	struct attribute *const *a;
+	const struct bin_attribute *const *ba;
+	struct kernfs_node *kn;
+	bool exists;
+	int i;
+
+	if (!kobj || !kobj->sd || !grp)
+		return false;
+
+	if (grp->name) {
+		kn = kernfs_find_and_get(kobj->sd, grp->name);
+		if (!kn)
+			return false;
+		exists = kernfs_type(kn) == KERNFS_DIR;
+		kernfs_put(kn);
+		return exists;
+	}
+
+	if (grp->attrs) {
+		for (i = 0, a = grp->attrs; *a; i++, a++) {
+			if (grp->is_visible && !grp->is_visible(kobj, *a, i))
+				continue;
+			kn = kernfs_find_and_get(kobj->sd, (*a)->name);
+			if (!kn)
+				return false;
+			kernfs_put(kn);
+		}
+	}
+	if (grp->bin_attrs) {
+		for (i = 0, ba = grp->bin_attrs; *ba; i++, ba++) {
+			if (grp->is_bin_visible && !grp->is_bin_visible(kobj, *ba, i))
+				continue;
+			kn = kernfs_find_and_get(kobj->sd, (*ba)->attr.name);
+			if (!kn)
+				return false;
+			kernfs_put(kn);
+		}
+	}
+	return true;
+}
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index b1a3a1e6ad09c..de211563f3dec 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -428,6 +428,8 @@ int __must_check sysfs_create_bin_file(struct kobject *kobj,
 void sysfs_remove_bin_file(struct kobject *kobj,
 			   const struct bin_attribute *attr);
 
+bool sysfs_kn_exists(struct kobject *kobj, const char *name);
+
 int __must_check sysfs_create_link(struct kobject *kobj, struct kobject *target,
 				   const char *name);
 int __must_check sysfs_create_link_nowarn(struct kobject *kobj,
@@ -454,6 +456,8 @@ void sysfs_remove_group(struct kobject *kobj,
 			const struct attribute_group *grp);
 void sysfs_remove_groups(struct kobject *kobj,
 			 const struct attribute_group *const *groups);
+bool sysfs_group_exists(struct kobject *kobj,
+			const struct attribute_group *grp);
 int sysfs_add_file_to_group(struct kobject *kobj,
 			const struct attribute *attr, const char *group);
 void sysfs_remove_file_from_group(struct kobject *kobj,
@@ -593,6 +597,11 @@ static inline void sysfs_remove_bin_file(struct kobject *kobj,
 {
 }
 
+static inline bool sysfs_kn_exists(struct kobject *kobj, const char *name)
+{
+	return false;
+}
+
 static inline int sysfs_create_link(struct kobject *kobj,
 				    struct kobject *target, const char *name)
 {
@@ -656,6 +665,12 @@ static inline void sysfs_remove_groups(struct kobject *kobj,
 {
 }
 
+static inline bool sysfs_group_exists(struct kobject *kobj,
+				      const struct attribute_group *grp)
+{
+	return false;
+}
+
 static inline int sysfs_add_file_to_group(struct kobject *kobj,
 		const struct attribute *attr, const char *group)
 {
-- 
2.47.3




Amazon Web Services Development Center Germany GmbH
Tamara-Danz-Str. 13
10243 Berlin
Geschaeftsfuehrung: Christof Hellmis, Andreas Stieger
Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
Sitz: Berlin
Ust-ID: DE 365 538 597
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.