sysfs host name support

PATCH In review Kurt Garloff 3 participants 7 messages Feb 9, 2004 – Feb 29, 2004 SCSI

lore

Share & tools

lore

Kurt Garloff

author

2004-02-09 15:50 UTC #0 lore raw

PATCH 1/1 sysfs host name support

Hi,

in scsidev, the host adapter name is one of the possible attributes to match when providing persistent device names.
And the module name is useful to deduce the list of modules that is needed when booting.
The proc_name was used previously for /proc/scsi/PROC_NAME/X/ and is still part of the host template. We might consider ripping is out or think about syncing it with the module name.

Unfortunately neither is visible in sysfs currently.

Please apply the attached patch.
It exports the 3 names to sysfs under /sys/classes/scsi_hosts/hostX/ name, proc_name, module_name.

Regards,

Signature · 2 lines

Kurt Garloff  <[email protected]>                            Cologne, DE 
SUSE LINUX AG, Nuernberg, DE                          SUSE Labs (Head)

scsi/scsi_sysfs.c renamed from scsi/scsi_sysfs.c.orig +15 -6

@@ -58,21 +58,24 @@
  * shost_show_function: macro to create an attr function that can be used to
  * show a non-bit field.
  */
-#define shost_show_function(field, format_string)			\
+#define shost_show_function(name, field, format_string)			\
 static ssize_t								\
-show_##field (struct class_device *class_dev, char *buf)		\
+show_##name (struct class_device *class_dev, char *buf)			\
 {									\
 	struct Scsi_Host *shost = class_to_shost(class_dev);		\
-	return snprintf (buf, 20, format_string, shost->field);	\
+	return snprintf (buf, 20, format_string, shost->field);		\
 }
 
 /*
  * shost_rd_attr: macro to create a function and attribute variable for a
  * read only field.
  */
-#define shost_rd_attr(field, format_string)				\
-	shost_show_function(field, format_string)			\
-static CLASS_DEVICE_ATTR(field, S_IRUGO, show_##field, NULL)
+#define shost_rd_attr2(name, field, format_string)			\
+	shost_show_function(name, field, format_string)			\
+static CLASS_DEVICE_ATTR(name, S_IRUGO, show_##name, NULL)
+
+#define shost_rd_attr(field, format_string) \
+shost_rd_attr2(field, field, format_string)
 
 /*
  * Create the actual show/store functions and data structures.

@@ -96,6 +99,9 @@
 shost_rd_attr(cmd_per_lun, "%hd\n");
 shost_rd_attr(sg_tablesize, "%hu\n");
 shost_rd_attr(unchecked_isa_dma, "%d\n");
+shost_rd_attr2(name, hostt->name, "%s\n");
+shost_rd_attr2(proc_name, hostt->proc_name, "%s\n");
+shost_rd_attr2(module_name, hostt->module->name, "%s\n");
 
 static struct class_device_attribute *scsi_sysfs_shost_attrs[] = {
 	&class_device_attr_unique_id,

@@ -103,6 +109,9 @@
 	&class_device_attr_cmd_per_lun,
 	&class_device_attr_sg_tablesize,
 	&class_device_attr_unchecked_isa_dma,
+	&class_device_attr_name,
+	&class_device_attr_proc_name,
+	&class_device_attr_module_name,
 	&class_device_attr_scan,
 	NULL
 };

Christoph Hellwig

2004-02-09 16:05 UTC #1 lore raw

Quoted 3 lines · Kurt Garloff

Please apply the attached patch.
It exports the 3 names to sysfs under /sys/classes/scsi_hosts/hostX/ name, proc_name, module_name.

name is optional (you can use ->info instead), so please don't export it. As for proc_name vs module_name - I think we should just export one of them, I'm pretty open which one. proc_name probably makes more sense.

Quoted 1 line

+shost_rd_attr2(module_name, hostt->module->name, "%s\n");

How does this work for builtin modules? Last time I checked hostt->module was NULL for those.

James Bottomley

2004-02-09 16:09 UTC #2 lore raw

Quoted 3 lines · Christoph Hellwig

name is optional (you can use ->info instead), so please don't export it. As for proc_name vs module_name - I think we should just export one of them, I'm pretty open which one. proc_name probably makes more sense.

Actually, no, I'd like to see module name as well.

We need a more generic than SCSI solution for this, I think, but if you look at most initrd systems, they go through a lot of pattern matching to try to translate the proc name into the module name. This would allow every initrd maker to eliminate those tiresome tables.

James

Christoph Hellwig

2004-02-09 16:20 UTC #3 lore raw

Quoted 7 lines · James Bottomley

Quoted 3 lines · Christoph Hellwig
name is optional (you can use ->info instead), so please don't export it. As for proc_name vs module_name - I think we should just export one of them, I'm pretty open which one. proc_name probably makes more sense.
Actually, no, I'd like to see module name as well. We need a more generic than SCSI solution for this, I think,

Agreed, this makes sense in common code. In fact I remember the driver model people were talking about it a while ago.

Kurt Garloff

author

2004-02-10 00:39 UTC #4 lore raw

Hi,

Quoted 6 lines · Christoph Hellwig

Quoted 2 lines · James Bottomley
Actually, no, I'd like to see module name as well. We need a more generic than SCSI solution for this, I think,
Agreed, this makes sense in common code. In fact I remember the driver model people were talking about it a while ago.

OK, let's export the proc_name then. It's some name that can be used to identify the driver, works for code both compiled statically or as module, is reasonably short and very often resembles the module name.

For the module_name, let's wait for a generic sysfs solution.

Please merge the attached patch.

Regards,

Signature · 2 lines

Kurt Garloff  <[email protected]>                            Cologne, DE 
SUSE LINUX AG, Nuernberg, DE                          SUSE Labs (Head)

scsi/scsi_sysfs.c renamed from scsi/scsi_sysfs.c.orig +11 -6

@@ -58,21 +58,24 @@
  * shost_show_function: macro to create an attr function that can be used to
  * show a non-bit field.
  */
-#define shost_show_function(field, format_string)			\
+#define shost_show_function(name, field, format_string)			\
 static ssize_t								\
-show_##field (struct class_device *class_dev, char *buf)		\
+show_##name (struct class_device *class_dev, char *buf)			\
 {									\
 	struct Scsi_Host *shost = class_to_shost(class_dev);		\
-	return snprintf (buf, 20, format_string, shost->field);	\
+	return snprintf (buf, 20, format_string, shost->field);		\
 }
 
 /*
  * shost_rd_attr: macro to create a function and attribute variable for a
  * read only field.
  */
-#define shost_rd_attr(field, format_string)				\
-	shost_show_function(field, format_string)			\
-static CLASS_DEVICE_ATTR(field, S_IRUGO, show_##field, NULL)
+#define shost_rd_attr2(name, field, format_string)			\
+	shost_show_function(name, field, format_string)			\
+static CLASS_DEVICE_ATTR(name, S_IRUGO, show_##name, NULL)
+
+#define shost_rd_attr(field, format_string) \
+shost_rd_attr2(field, field, format_string)
 
 /*
  * Create the actual show/store functions and data structures.

@@ -96,6 +99,7 @@
 shost_rd_attr(cmd_per_lun, "%hd\n");
 shost_rd_attr(sg_tablesize, "%hu\n");
 shost_rd_attr(unchecked_isa_dma, "%d\n");
+shost_rd_attr2(proc_name, hostt->proc_name, "%s\n");
 
 static struct class_device_attribute *scsi_sysfs_shost_attrs[] = {
 	&class_device_attr_unique_id,

@@ -103,6 +107,7 @@
 	&class_device_attr_cmd_per_lun,
 	&class_device_attr_sg_tablesize,
 	&class_device_attr_unchecked_isa_dma,
+	&class_device_attr_proc_name,
 	&class_device_attr_scan,
 	NULL
 };

Kurt Garloff

author

2004-02-29 23:37 UTC #5 lore raw

Quoted 8 lines · Kurt Garloff

OK, let's export the proc_name then. It's some name that can be used to identify the driver, works for code both compiled statically or as module, is reasonably short and very often resembles the module name.
For the module_name, let's wait for a generic sysfs solution.
Please merge the attached patch.

Any consensus about this one?

I'd certainly like to see it merged.

Regards,

Signature · 2 lines

Kurt Garloff  <[email protected]>                            Cologne, DE 
SUSE LINUX AG, Nuernberg, DE                          SUSE Labs (Head)

drivers/scsi/scsi_sysfs.c renamed from drivers/scsi/scsi_sysfs.c.orig +11 -6

@@ -58,21 +58,24 @@
  * shost_show_function: macro to create an attr function that can be used to
  * show a non-bit field.
  */
-#define shost_show_function(field, format_string)			\
+#define shost_show_function(name, field, format_string)			\
 static ssize_t								\
-show_##field (struct class_device *class_dev, char *buf)		\
+show_##name (struct class_device *class_dev, char *buf)			\
 {									\
 	struct Scsi_Host *shost = class_to_shost(class_dev);		\
-	return snprintf (buf, 20, format_string, shost->field);	\
+	return snprintf (buf, 20, format_string, shost->field);		\
 }
 
 /*
  * shost_rd_attr: macro to create a function and attribute variable for a
  * read only field.
  */
-#define shost_rd_attr(field, format_string)				\
-	shost_show_function(field, format_string)			\
-static CLASS_DEVICE_ATTR(field, S_IRUGO, show_##field, NULL)
+#define shost_rd_attr2(name, field, format_string)			\
+	shost_show_function(name, field, format_string)			\
+static CLASS_DEVICE_ATTR(name, S_IRUGO, show_##name, NULL)
+
+#define shost_rd_attr(field, format_string) \
+shost_rd_attr2(field, field, format_string)
 
 /*
  * Create the actual show/store functions and data structures.

@@ -96,6 +99,7 @@
 shost_rd_attr(cmd_per_lun, "%hd\n");
 shost_rd_attr(sg_tablesize, "%hu\n");
 shost_rd_attr(unchecked_isa_dma, "%d\n");
+shost_rd_attr2(proc_name, hostt->proc_name, "%s\n");
 
 static struct class_device_attribute *scsi_sysfs_shost_attrs[] = {
 	&class_device_attr_unique_id,

@@ -103,6 +107,7 @@
 	&class_device_attr_cmd_per_lun,
 	&class_device_attr_sg_tablesize,
 	&class_device_attr_unchecked_isa_dma,
+	&class_device_attr_proc_name,
 	&class_device_attr_scan,
 	NULL
 };

Kurt Garloff

author

2004-02-09 16:20 UTC #6 lore raw

In reply to Christoph Hellwig

Hi Christoph,

Quoted 6 lines · Christoph Hellwig

Quoted 3 lines · Kurt Garloff
Please apply the attached patch.
It exports the 3 names to sysfs under /sys/classes/scsi_hosts/hostX/ name, proc_name, module_name.
name is optional (you can use ->info instead), so please don't export it.

Hmm, I think it's still mandatory, except that it will be overridden by info(). It's still useful info in any case, I'd still like to export it.

Quoted 2 lines

As for proc_name vs module_name - I think we should just export one of them, I'm pretty open which one. proc_name probably makes more sense.

I can do with just one name.

Normally, the module name would look most useful, as it can be used by installers etc. who need to figure what modules are needed. I would prefer if proc_name would be identical to module name, and then export the proc_name, which would even work for statically compiled in drivers.

Quoted 4 lines

Quoted 1 line
+shost_rd_attr2(module_name, hostt->module->name, "%s\n");
How does this work for builtin modules? Last time I checked hostt->module was NULL for those.

Nice null-ptr deref Oops. Looks like we need to handcode a function for this one. Maybe with fallback to proc_name ...

Regards,

Signature · 2 lines

Kurt Garloff  <[email protected]>                            Cologne, DE 
SUSE LINUX AG, Nuernberg, DE                          SUSE Labs (Head)