[PATCH] drivers: scsi: alloc scsi disk ida index before adding scsi device

[PATCH] drivers: scsi: alloc scsi disk ida index before adding scsi device

[Jianguo Sun]

Originally, we alloc ida index in scsi driver(sd_probe) asynchronously,
which may cause disk name out of order and in turn cause rootfs mount
issue if we set specific disk mount as the root filesystem by parameters
"root=/dev/sda6", for example.
Since the scsi device gets added in sequence, so we change to alloc ida
index for scsi disk before adding scsi device.

Signed-off-by: Jianguo Sun <quic_jianguos@quicinc.com>
---
 drivers/scsi/scsi_sysfs.c  | 14 ++++++++++++++
 drivers/scsi/sd.c          | 36 ++++++++++++++++++++++++------------
 include/scsi/scsi_device.h |  1 +
 3 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 226a50944c00..7682e31ecd58 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -10,6 +10,7 @@
 #include <linux/module.h>
 #include <linux/slab.h>
 #include <linux/init.h>
+#include <linux/idr.h>
 #include <linux/blkdev.h>
 #include <linux/device.h>
 #include <linux/pm_runtime.h>
@@ -28,6 +29,8 @@
 #include "scsi_logging.h"
 
 static struct device_type scsi_dev_type;
+extern struct ida sd_index_ida;
+extern bool is_sd_type_supported(struct scsi_device *sdp);
 
 static const struct {
 	enum scsi_device_state	value;
@@ -1363,6 +1366,17 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev)
 
 	scsi_dh_add_device(sdev);
 
+	/**
+	 * Originally, we alloc ida index in scsi driver(sd_probe) asynchronously,
+	 * which may cause disk name out of order and in turn cause rootfs mount
+	 * issue if we set specific disk mount as the root filesystem by parameters
+	 * "root=/dev/sda6", for example.
+	 * Since the scsi device gets added in sequence, so we change to alloc ida
+	 * index for scsi disk before adding scsi device.
+	 **/
+	if (is_sd_type_supported(sdev))
+		sdev->sd_index = ida_alloc(&sd_index_ida, GFP_KERNEL);
+
 	error = device_add(&sdev->sdev_gendev);
 	if (error) {
 		sdev_printk(KERN_INFO, sdev,
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index a390679cf458..8c39280f0eeb 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -119,7 +119,7 @@ static int sd_eh_action(struct scsi_cmnd *, int);
 static void sd_read_capacity(struct scsi_disk *sdkp, unsigned char *buffer);
 static void scsi_disk_release(struct device *cdev);
 
-static DEFINE_IDA(sd_index_ida);
+DEFINE_IDA(sd_index_ida);
 
 static struct kmem_cache *sd_cdb_cache;
 static mempool_t *sd_page_pool;
@@ -3346,6 +3346,26 @@ static int sd_format_disk_name(char *prefix, int index, char *buf, int buflen)
 	return 0;
 }
 
+inline bool is_sd_type_supported(struct scsi_device *sdp)
+{
+	bool ret = false;
+
+	sdev_printk(KERN_DEBUG, sdp,
+			"sdp->type: 0x%x\n", sdp->type);
+
+#ifndef CONFIG_BLK_DEV_ZONED
+	if (sdp->type == TYPE_ZBC)
+		ret = false;
+#endif
+	if (sdp->type == TYPE_DISK ||
+	    sdp->type == TYPE_ZBC ||
+	    sdp->type == TYPE_MOD ||
+	    sdp->type == TYPE_RBC)
+		ret = true;
+
+	return ret;
+}
+
 /**
  *	sd_probe - called during driver initialization and whenever a
  *	new scsi device is attached to the system. It is called once
@@ -3374,18 +3394,9 @@ static int sd_probe(struct device *dev)
 
 	scsi_autopm_get_device(sdp);
 	error = -ENODEV;
-	if (sdp->type != TYPE_DISK &&
-	    sdp->type != TYPE_ZBC &&
-	    sdp->type != TYPE_MOD &&
-	    sdp->type != TYPE_RBC)
+	if (!is_sd_type_supported(sdp))
 		goto out;
 
-	if (!IS_ENABLED(CONFIG_BLK_DEV_ZONED) && sdp->type == TYPE_ZBC) {
-		sdev_printk(KERN_WARNING, sdp,
-			    "Unsupported ZBC host-managed device.\n");
-		goto out;
-	}
-
 	SCSI_LOG_HLQUEUE(3, sdev_printk(KERN_INFO, sdp,
 					"sd_probe\n"));
 
@@ -3399,7 +3410,7 @@ static int sd_probe(struct device *dev)
 	if (!gd)
 		goto out_free;
 
-	index = ida_alloc(&sd_index_ida, GFP_KERNEL);
+	index = sdp->sd_index;
 	if (index < 0) {
 		sdev_printk(KERN_WARNING, sdp, "sd_probe: memory exhausted.\n");
 		goto out_put;
@@ -3410,6 +3421,7 @@ static int sd_probe(struct device *dev)
 		sdev_printk(KERN_WARNING, sdp, "SCSI disk (sd) name length exceeded.\n");
 		goto out_free_index;
 	}
+	sdev_printk(KERN_INFO, sdp, "sd_probe: index %d, [%s]\n", index, gd->disk_name);
 
 	sdkp->device = sdp;
 	sdkp->disk = gd;
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 57e3e239a1fc..23b52481b84c 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -246,6 +246,7 @@ struct scsi_device {
 	enum scsi_device_state sdev_state;
 	struct task_struct	*quiesced_by;
 	unsigned long		sdev_data[];
+	long			sd_index; /* scsi disk index */
 } __attribute__((aligned(sizeof(unsigned long))));
 
 #define	to_scsi_device(d)	\
-- 
2.17.1

Re: [PATCH] drivers: scsi: alloc scsi disk ida index before adding scsi device

[Bart Van Assche]

On 3/31/22 17:59, Jianguo Sun wrote:
> Originally, we alloc ida index in scsi driver(sd_probe) asynchronously,
> which may cause disk name out of order and in turn cause rootfs mount
> issue if we set specific disk mount as the root filesystem by parameters
> "root=/dev/sda6", for example.
> Since the scsi device gets added in sequence, so we change to alloc ida
> index for scsi disk before adding scsi device.

Specifying a /dev/sd* device name as root device is wrong since such a 
name will change if a disk is added to the system or removed from the 
system. Please use one of the /dev/disk/by-*/* names as root device.

Thanks,

Bart.