sysfs: allow the group is_visible() method to return a mode and add an update API

We have a problem in scsi_transport_spi in that we need to customise not only the visibility of the attributes, but also their mode. Fix this by making the is_visible() callback return a mode, with 0 indicating is not visible.

Also add a sysfs_update_group() API to allow us to change either the visibility or mode of the files at any time on the fly.

Signed-off-by James Bottomley <[email protected]>

---

Greg, can I take this through SCSI, please ... assuming it's OK? We need this to fix up the SPI transport class.

Quoted 8 lines · James Bottomley

We have a problem in scsi_transport_spi in that we need to customise not only the visibility of the attributes, but also their mode. Fix this by making the is_visible() callback return a mode, with 0 indicating is not visible.

Also add a sysfs_update_group() API to allow us to change either the visibility or mode of the files at any time on the fly.

Quoted 5 lines

Signed-off-by James Bottomley <[email protected]>

---

Greg, can I take this through SCSI, please ... assuming it's OK? We need this to fix up the SPI transport class.

Sure. But can you document that "update" field a bit more so we can remember what it means in the future?

Quoted 12 lines · Greg KH

Quoted 11 lines · James Bottomley

We have a problem in scsi_transport_spi in that we need to customise not only the visibility of the attributes, but also their mode. Fix this by making the is_visible() callback return a mode, with 0 indicating is not visible.

Also add a sysfs_update_group() API to allow us to change either the visibility or mode of the files at any time on the fly.

Signed-off-by James Bottomley <[email protected]>

---

Quoted 5 lines

Quoted 2 lines

Greg, can I take this through SCSI, please ... assuming it's OK? We need this to fix up the SPI transport class.

Sure. But can you document that "update" field a bit more so we can remember what it means in the future?

How about this? I also added docbook for create and update attribute group.

James

---

From: James Bottomley <[email protected]>
Date: Thu, 20 Mar 2008 20:47:52 -0500
Subject: [SCSI] sysfs: make group is_valid return a mode_t

We have a problem in scsi_transport_spi in that we need to customise not only the visibility of the attributes, but also their mode. Fix this by making the is_visible() callback return a mode, with 0 indicating is not visible.

Also add a sysfs_update_group() API to allow us to change either the visibility or mode of the files at any time on the fly.

Signed-off-by James Bottomley <[email protected]>

---
 fs/sysfs/file.c       |   14 ++++++--
 fs/sysfs/group.c      |   77 +++++++++++++++++++++++++++++++++++++++++-------
 fs/sysfs/sysfs.h      |    2 +
 include/linux/sysfs.h |    4 ++-
 4 files changed, 80 insertions(+), 17 deletions(-)

fs/sysfs/file.c +10 -4

index 6f5954a..8f7ee10 100644
@@ -490,11 +490,10 @@ const struct file_operations sysfs_file_operations = {
 	.poll		= sysfs_poll,
 };
 
-
-int sysfs_add_file(struct sysfs_dirent *dir_sd, const struct attribute *attr,
-		   int type)
+int sysfs_add_file_mode(struct sysfs_dirent *dir_sd,
+			const struct attribute *attr, int type, mode_t amode)
 {
-	umode_t mode = (attr->mode & S_IALLUGO) | S_IFREG;
+	umode_t mode = (amode & S_IALLUGO) | S_IFREG;
 	struct sysfs_addrm_cxt acxt;
 	struct sysfs_dirent *sd;
 	int rc;

@@ -515,6 +514,13 @@ int sysfs_add_file(struct sysfs_dirent *dir_sd, const struct attribute *attr,
 }
 
 
+int sysfs_add_file(struct sysfs_dirent *dir_sd, const struct attribute *attr,
+		   int type)
+{
+	return sysfs_add_file_mode(dir_sd, attr, type, attr->mode);
+}
+
+
 /**
  *	sysfs_create_file - create an attribute file for an object.
  *	@kobj:	object we're creating for. 

fs/sysfs/group.c +65 -12

index 4779049..dac9383 100644
@@ -23,30 +23,41 @@ static void remove_files(struct sysfs_dirent *dir_sd, struct kobject *kobj,
 	int i;
 
 	for (i = 0, attr = grp->attrs; *attr; i++, attr++)
-		if (!grp->is_visible ||
-		    grp->is_visible(kobj, *attr, i))
-			sysfs_hash_and_remove(dir_sd, (*attr)->name);
+		sysfs_hash_and_remove(dir_sd, (*attr)->name);
 }
 
 static int create_files(struct sysfs_dirent *dir_sd, struct kobject *kobj,
-			const struct attribute_group *grp)
+			const struct attribute_group *grp, int update)
 {
 	struct attribute *const* attr;
 	int error = 0, i;
 
-	for (i = 0, attr = grp->attrs; *attr && !error; i++, attr++)
-		if (!grp->is_visible ||
-		    grp->is_visible(kobj, *attr, i))
-			error |=
-				sysfs_add_file(dir_sd, *attr, SYSFS_KOBJ_ATTR);
+	for (i = 0, attr = grp->attrs; *attr && !error; i++, attr++) {
+		mode_t mode = 0;
+
+		/* in update mode, we're changing the permissions or
+		 * visibility.  Do this by first removing then
+		 * re-adding (if required) the file */
+		if (update)
+			sysfs_hash_and_remove(dir_sd, (*attr)->name);
+		if (grp->is_visible) {
+			mode = grp->is_visible(kobj, *attr, i);
+			if (!mode)
+				continue;
+		}
+		error = sysfs_add_file_mode(dir_sd, *attr, SYSFS_KOBJ_ATTR,
+					    (*attr)->mode | mode);
+		if (unlikely(error))
+			break;
+	}
 	if (error)
 		remove_files(dir_sd, kobj, grp);
 	return error;
 }
 
 
-int sysfs_create_group(struct kobject * kobj, 
-		       const struct attribute_group * grp)
+static int internal_create_group(struct kobject *kobj, int update,
+				 const struct attribute_group *grp)
 {
 	struct sysfs_dirent *sd;
 	int error;

@@ -60,7 +71,7 @@ int sysfs_create_group(struct kobject * kobj,
 	} else
 		sd = kobj->sd;
 	sysfs_get(sd);
-	error = create_files(sd, kobj, grp);
+	error = create_files(sd, kobj, grp, update);
 	if (error) {
 		if (grp->name)
 			sysfs_remove_subdir(sd);

@@ -69,6 +80,47 @@ int sysfs_create_group(struct kobject * kobj,
 	return error;
 }
 
+/**
+ * sysfs_create_group - given a directory kobject, create an attribute group
+ * @kobj:	The kobject to create the group on
+ * @grp:	The attribute group to create
+ *
+ * This function creates a group for the first time.  It will explicitly
+ * warn and error if any of the attribute files being created already exist.
+ *
+ * Returns 0 on success or error.
+ */
+int sysfs_create_group(struct kobject *kobj,
+		       const struct attribute_group *grp)
+{
+	return internal_create_group(kobj, 0, grp);
+}
+
+/**
+ * sysfs_update_group - given a directory kobject, create an attribute group
+ * @kobj:	The kobject to create the group on
+ * @grp:	The attribute group to create
+ *
+ * This function updates an attribute group.  Unlike
+ * sysfs_create_group(), it will explicitly not warn or error if any
+ * of the attribute files being created already exist.  Furthermore,
+ * if the visibility of the files has changed through the is_visible()
+ * callback, it will update the permissions and add or remove the
+ * relevant files.
+ *
+ * The primary use for this function is to call it after making a change
+ * that affects group visibility.
+ *
+ * Returns 0 on success or error.
+ */
+int sysfs_update_group(struct kobject *kobj,
+		       const struct attribute_group *grp)
+{
+	return internal_create_group(kobj, 1, grp);
+}
+
+
+
 void sysfs_remove_group(struct kobject * kobj, 
 			const struct attribute_group * grp)
 {

@@ -95,4 +147,5 @@ void sysfs_remove_group(struct kobject * kobj,
 
 
 EXPORT_SYMBOL_GPL(sysfs_create_group);
+EXPORT_SYMBOL_GPL(sysfs_update_group);
 EXPORT_SYMBOL_GPL(sysfs_remove_group);

fs/sysfs/sysfs.h +2 -0

index 52aaa8c..2915959 100644
@@ -155,6 +155,8 @@ extern const struct file_operations sysfs_file_operations;
 int sysfs_add_file(struct sysfs_dirent *dir_sd,
 		   const struct attribute *attr, int type);
 
+int sysfs_add_file_mode(struct sysfs_dirent *dir_sd,
+			const struct attribute *attr, int type, mode_t amode);
 /*
  * bin.c
  */

include/linux/sysfs.h +3 -1

index 0f388af..07d9c12 100644
@@ -32,7 +32,7 @@ struct attribute {
 
 struct attribute_group {
 	const char		*name;
-	int			(*is_visible)(struct kobject *,
+	mode_t			(*is_visible)(struct kobject *,
 					      struct attribute *, int);
 	struct attribute	**attrs;
 };

@@ -105,6 +105,8 @@ void sysfs_remove_link(struct kobject *kobj, const char *name);
 
 int __must_check sysfs_create_group(struct kobject *kobj,
 				    const struct attribute_group *grp);
+int sysfs_update_group(struct kobject *kobj,
+		       const struct attribute_group *grp);
 void sysfs_remove_group(struct kobject *kobj,
 			const struct attribute_group *grp);
 int sysfs_add_file_to_group(struct kobject *kobj,

Quoted 13 lines · James Bottomley

Quoted 12 lines · Greg KH

Quoted 11 lines · James Bottomley

We have a problem in scsi_transport_spi in that we need to customise not only the visibility of the attributes, but also their mode. Fix this by making the is_visible() callback return a mode, with 0 indicating is not visible.

Also add a sysfs_update_group() API to allow us to change either the visibility or mode of the files at any time on the fly.

Signed-off-by James Bottomley <[email protected]>

---

Quoted 8 lines

Quoted 5 lines

Quoted 2 lines

Greg, can I take this through SCSI, please ... assuming it's OK? We need this to fix up the SPI transport class.

Sure. But can you document that "update" field a bit more so we can remember what it means in the future?

How about this? I also added docbook for create and update attribute group.

Can I get an ACK from someone from sysfs for this, please?

Quoted 13 lines

---

From: James Bottomley <[email protected]>
Date: Thu, 20 Mar 2008 20:47:52 -0500
Subject: [SCSI] sysfs: make group is_valid return a mode_t

We have a problem in scsi_transport_spi in that we need to customise not only the visibility of the attributes, but also their mode. Fix this by making the is_visible() callback return a mode, with 0 indicating is not visible.

Also add a sysfs_update_group() API to allow us to change either the visibility or mode of the files at any time on the fly.

Quoted 1 line

Signed-off-by James Bottomley <[email protected]>

Quoted 14 lines · James Bottomley

Quoted 13 lines · James Bottomley

Quoted 12 lines · Greg KH

Quoted 11 lines · James Bottomley

We have a problem in scsi_transport_spi in that we need to customise not only the visibility of the attributes, but also their mode. Fix this by making the is_visible() callback return a mode, with 0 indicating is not visible.

Also add a sysfs_update_group() API to allow us to change either the visibility or mode of the files at any time on the fly.

Signed-off-by James Bottomley <[email protected]>

---

Quoted 8 lines

Quoted 8 lines

Quoted 5 lines

Quoted 2 lines

Greg, can I take this through SCSI, please ... assuming it's OK? We need this to fix up the SPI transport class.

Sure. But can you document that "update" field a bit more so we can remember what it means in the future?

How about this? I also added docbook for create and update attribute group.

Looks good to me.

I think with the "visible" stuff for attribute groups, will be able to fix the USB event timing problem, by moving the creation of the device attributes to default attributes. So I'm looking forward to have this in the tree.

Quoted 13 lines

Can I get an ACK from someone from sysfs for this, please?

Quoted 11 lines

---

From: James Bottomley <[email protected]>
Date: Thu, 20 Mar 2008 20:47:52 -0500
Subject: [SCSI] sysfs: make group is_valid return a mode_t

We have a problem in scsi_transport_spi in that we need to customise not only the visibility of the attributes, but also their mode. Fix this by making the is_visible() callback return a mode, with 0 indicating is not visible.

Quoted 2 lines

Quoted 2 lines

Also add a sysfs_update_group() API to allow us to change either the visibility or mode of the files at any time on the fly.

Acked-by Kay Sievers <[email protected]>