scsi: core: Fix early registration of sysfs attributes for scsi_device

v4.17 commit 86b87cde0b55 ("scsi: core: host template attribute groups")
introduced explicit sysfs_create_groups() in scsi_sysfs_add_sdev()
and sysfs_remove_groups() in __scsi_remove_device(), both for sdev_gendev,
based on a new field const struct attribute_group **sdev_groups
of struct scsi_host_template.

Commit 92c4b58b15c5 ("scsi: core: Register sysfs attributes earlier")
removed above explicit (de)registration of scsi_device attribute groups.
It also converted all scsi_device attributes and attribute_groups to
end up in a new field const struct attribute_group *gendev_attr_groups[6]
of struct scsi_device. However, that new field was not used anywhere.

Surprisingly, this only caused missing LLDD specific scsi_device sysfs
attributes. Whereas, scsi core attributes from scsi_sdev_attr_groups
did continue to exist because of scsi_dev_type.groups.

We separate scsi core attibutes from LLDD specific attributes.
Hence, we keep the initializing assignment scsi_dev_type =
{ .groups = scsi_sdev_attr_groups, } as this takes care of core
attributes. Without the separation, it would cause attribute double
registration due to scsi_dev_type.groups and sdev_gendev.groups.

Julian suggested to assign the sdev_groups pointer of the
scsi_host_template directly to the groups pointer of sdev_gendev.
This way we can delete the container scsi_device.gendev_attr_groups
and the loop copying each entry from hostt->sdev_groups to
sdev->gendev_attr_groups.

Alternative approaches ruled out:
Assigning gendev_attr_groups to sdev_dev has no visible effect.
Assigning sdev->gendev_attr_groups to scsi_dev_type.groups
caused scsi_device of all scsi host types to get LLDD specific
attributes of the LLDD for which the last sdev alloc happened to occur,
as that overwrote scsi_dev_type.groups,
e.g. scsi_debug had zfcp-specific scsi_device attributes.

Link: https://lore.kernel.org/r/20211026014240.4098365-1-maier@linux.ibm.com
Fixes: 92c4b58b15c5 ("scsi: core: Register sysfs attributes earlier")
Suggested-by: Julian Wiedmann <jwi@linux.ibm.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Steffen Maier <maier@linux.ibm.com>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index c26f0e29e8cd2b89fb3f770fc3872c794cc6b2cc..fa064bf9a55cdc94ca122d0716f27c296fcdf6d8 100644 (file)
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1571,7 +1571,6 @@ static struct device_type scsi_dev_type = {
 
 void scsi_sysfs_device_initialize(struct scsi_device *sdev)
 {
-       int i, j = 0;
        unsigned long flags;
        struct Scsi_Host *shost = sdev->host;
        struct scsi_host_template *hostt = shost->hostt;
@@ -1583,15 +1582,7 @@ void scsi_sysfs_device_initialize(struct scsi_device *sdev)
        scsi_enable_async_suspend(&sdev->sdev_gendev);
        dev_set_name(&sdev->sdev_gendev, "%d:%d:%d:%llu",
                     sdev->host->host_no, sdev->channel, sdev->id, sdev->lun);
-       sdev->gendev_attr_groups[j++] = &scsi_sdev_attr_group;
-       if (hostt->sdev_groups) {
-               for (i = 0; hostt->sdev_groups[i] &&
-                            j < ARRAY_SIZE(sdev->gendev_attr_groups);
-                    i++, j++) {
-                       sdev->gendev_attr_groups[j] = hostt->sdev_groups[i];
-               }
-       }
-       WARN_ON_ONCE(j >= ARRAY_SIZE(sdev->gendev_attr_groups));
+       sdev->sdev_gendev.groups = hostt->sdev_groups;
 
        device_initialize(&sdev->sdev_dev);
        sdev->sdev_dev.parent = get_device(&sdev->sdev_gendev);

diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index b1e9b3bd3a602f8e89eed4c3487a9b9b529c5213..b97e142a7ca92d5f45fa21374c2e627ff29afef5 100644 (file)
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -225,12 +225,6 @@ struct scsi_device {
 
        struct device           sdev_gendev,
                                sdev_dev;
-       /*
-        * The array size 6 provides space for one attribute group for the
-        * SCSI core, four attribute groups defined by SCSI LLDs and one
-        * terminating NULL pointer.
-        */
-       const struct attribute_group *gendev_attr_groups[6];
 
        struct execute_work     ew; /* used to get process context on put */
        struct work_struct      requeue_work;