Subject: Re: [PATCH v3] scsi: core: Fix early registration of sysfs attributes for scsi_device

From: Bart Van Assche <[email-masked]>
Date: Tue, 26 Oct 2021 14:48:55 -0700

On 10/26/21 3:00 AM, Benjamin Block wrote:
> Hmm, maybe this is out of scope for this fix, but couldn't we
> essentially do the same thing for the host attributes. Have the
> `shost_class` take the `scsi_shost_attr_group` as default attributes for
> the shost class, and then assign the `shost_groups` from the LLDD
> template to `shost_dev.groups` as optional attributes?
> 
> Then we get rid of the indirection loop in `hosts.c` as well, that was
> introduce with he original patch by Bart.
> 
> Just a shot in the dark, I don't know whether the `struct class` behaves
> the same in this case as `struct device_type`.

Is something like this what you have in mind?

Thanks,

Bart.

Subject: [PATCH] scsi: core: Remove Scsi_Host.shost_dev_attr_groups

Suggested-by: Benjamin Block <[email-masked]>
Signed-off-by: Bart Van Assche <[email-masked]>
---
  drivers/scsi/hosts.c      | 15 +++------------
  drivers/scsi/scsi_priv.h  |  2 +-
  drivers/scsi/scsi_sysfs.c |  7 ++++++-
  include/scsi/scsi_host.h  |  6 ------
  4 files changed, 10 insertions(+), 20 deletions(-)

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 17aef936bc90..f88e7ed77dbb 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -61,6 +61,7 @@ static void scsi_host_cls_release(struct device *dev)
  static struct class shost_class = {
  	.name		= "scsi_host",
  	.dev_release	= scsi_host_cls_release,
+	.dev_groups	= scsi_shost_groups,
  };

  /**
@@ -376,7 +377,7 @@ static struct device_type scsi_host_type = {
  struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
  {
  	struct Scsi_Host *shost;
-	int index, i, j = 0;
+	int index;

  	shost = kzalloc(sizeof(struct Scsi_Host) + privsize, GFP_KERNEL);
  	if (!shost)
@@ -481,17 +482,7 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
  	shost->shost_dev.parent = &shost->shost_gendev;
  	shost->shost_dev.class = &shost_class;
  	dev_set_name(&shost->shost_dev, "host%d", shost->host_no);
-	shost->shost_dev.groups = shost->shost_dev_attr_groups;
-	shost->shost_dev_attr_groups[j++] = &scsi_shost_attr_group;
-	if (sht->shost_groups) {
-		for (i = 0; sht->shost_groups[i] &&
-			     j < ARRAY_SIZE(shost->shost_dev_attr_groups);
-		     i++, j++) {
-			shost->shost_dev_attr_groups[j] =
-				sht->shost_groups[i];
-		}
-	}
-	WARN_ON_ONCE(j >= ARRAY_SIZE(shost->shost_dev_attr_groups));
+	shost->shost_dev.groups = sht->shost_groups;

  	shost->ehandler = kthread_run(scsi_error_handler, shost,
  			"scsi_eh_%d", shost->host_no);
diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index a278fc8948f4..0f5743f4769b 100644
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -144,7 +144,7 @@ extern struct scsi_transport_template blank_transport_template;
  extern void __scsi_remove_device(struct scsi_device *);

  extern struct bus_type scsi_bus_type;
-extern const struct attribute_group scsi_shost_attr_group;
+extern const struct attribute_group *scsi_shost_groups[];

  /* scsi_netlink.c */
  #ifdef CONFIG_SCSI_NETLINK
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index c26f0e29e8cd..f360154b5241 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -424,10 +424,15 @@ static struct attribute *scsi_sysfs_shost_attrs[] = {
  	NULL
  };

-const struct attribute_group scsi_shost_attr_group = {
+static const struct attribute_group scsi_shost_attr_group = {
  	.attrs =	scsi_sysfs_shost_attrs,
  };

+const struct attribute_group *scsi_shost_groups[] = {
+	&scsi_shost_attr_group,
+	NULL
+};
+
  static void scsi_device_cls_release(struct device *class_dev)
  {
  	struct scsi_device *sdev;
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index ae715959f886..97cdad14de56 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -690,12 +690,6 @@ struct Scsi_Host {

  	/* ldm bits */
  	struct device		shost_gendev, shost_dev;
-	/*
-	 * The array size 3 provides space for one attribute group defined by
-	 * the SCSI core, one attribute group defined by the SCSI LLD and one
-	 * terminating NULL pointer.
-	 */
-	const struct attribute_group *shost_dev_attr_groups[3];

  	/*
  	 * Points to the transport data (if any) which is allocated