When creating an attribute group, if it is named a subdirectory it is
created and the sysfs files are placed into that subdirectory. If no
files are created, normally the directory would still be present, but it
would be empty.
This can be confusing for users, as it appears the feature is avaliable
as there is a directory, but it isn't supported by the hardware or the
kernel.
One way to fix this is to remove directories that don't contain any
files, such as [1]. The problem with this is that there are currently
lots of users in the kernel who expect the group to remain even if
empty, as they dynamically add/merge properties later.
The documentation for sysfs_merge_group() specifically says
This function returns an error if the group doesn't exist or any of the
files already exist in that group, in which case none of the new files
are created.
So just not adding the group if it's empty doesn't work, at least not
with the code we currently have. The code can be changed to support
this, but it is difficult to determine how this will affect existing use
cases.
This approach instead adds a new function pointer, attr_is_visible(), to
`struct attribute_group` which can be set if the user wants to filter
the avaliablility of the function.
This matches the .is_visible() function pointer that already exists and
is commonly used. This approach provides greater control over if the
directory should be visible or not.
This will be used by the PCIe DOE sysfs attributes to kind the directory
on devices that don't support DOE.
1: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git/commit/?h=debugfs_cleanup&id=f670945dfbaf353fe068544c31e3fa45575da5b5
Signed-off-by: Alistair Francis <alistair.francis@xxxxxxx>
---
v6:
- Add patch
fs/sysfs/group.c | 12 +++++++++++-
include/linux/sysfs.h | 6 ++++++
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c
index 138676463336..34afd5becdbe 100644
--- a/fs/sysfs/group.c
+++ b/fs/sysfs/group.c
@@ -111,6 +111,7 @@ static int internal_create_group(struct kobject *kobj, int update,
kuid_t uid;
kgid_t gid;
int error;
+ umode_t mode;
if (WARN_ON(!kobj || (!update && !kobj->sd)))
return -EINVAL;
@@ -125,6 +126,15 @@ static int internal_create_group(struct kobject *kobj, int update,
return 0;
}
+ if (grp->attr_is_visible) {
+ mode = grp->attr_is_visible(kobj);
+
+ if (mode == 0)
+ return 0;
+ } else {
+ mode = S_IRWXU | S_IRUGO | S_IXUGO;
+ }
+
kobject_get_ownership(kobj, &uid, &gid);
if (grp->name) {
if (update) {
@@ -136,7 +146,7 @@ static int internal_create_group(struct kobject *kobj, int update,
}
} else {
kn = kernfs_create_dir_ns(kobj->sd, grp->name,
- S_IRWXU | S_IRUGO | S_IXUGO,
+ mode,
uid, gid, kobj, NULL);
if (IS_ERR(kn)) {
if (PTR_ERR(kn) == -EEXIST)
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index fd3fe5c8c17f..808e7fc0ca57 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -63,6 +63,11 @@ do { \
* @name: Optional: Attribute group name
* If specified, the attribute group will be created in
* a new subdirectory with this name.
+ * @attr_is_visible: Optional: Function to return permissions
+ * associated with the attribute group. Only read/write
+ * permissions as well as SYSFS_PREALLOC are accepted. Must
+ * return 0 if an attribute is not visible. The returned value
+ * will replace static permissions defined in struct attribute.
* @is_visible: Optional: Function to return permissions associated with an
* attribute of the group. Will be called repeatedly for each
* non-binary attribute in the group. Only read/write
@@ -83,6 +88,7 @@ do { \
*/
struct attribute_group {
const char *name;
+ umode_t (*attr_is_visible)(struct kobject *);
umode_t (*is_visible)(struct kobject *,
struct attribute *, int);
umode_t (*is_bin_visible)(struct kobject *,
--
2.41.0
