[PATCHv4 0/5] scsi: use xarray for devices and targets

[PATCHv4 0/5] scsi: use xarray for devices and targets

[Hannes Reinecke]

Hi all,

based on the ideas from Doug Gilbert here's now my take on using
xarrays for devices and targets.
It revolves around two ideas:

 - The scsi target 'channel' and 'id' numbers are never ever used
   to the full 32 bit range; channels are well below 10, and no
   driver is using more than 16 bits for the id. So we can reduce
   the type of 'channel' and 'id' to 16 bits, and use the 32 bit
   value 'channel << 16 | id' as the index into the target xarray.
 - Most SCSI targets are only using the first 32 bits of the 64 bit
   LUN structure. So there we can use the LUN number as the index into
   the xarray; larger LUN numbers won't fit, so we'll allocate a
   separate index for those. For these device lookup won't be as
   efficient, but one can argue that we're running into scalability
   issues long before that.

With these changes we can implement an efficient lookup mechanism,
devolving into direct lookup for most cases.
And iteration over devices should be as efficient as the current,
list-based, approach.

It also removes the current duality between host-based and
target-based device lists; now all devices are listed per target,
and a full iteration would need to iterate first over all targets
and then over all devices per target.

As usual, comments and reviews are welcome

Changes to v1:
- Fixup __scsi_iterate_devices()
- Include reviews from Johannes
- Minor fixes
- Include comments from Doug

Changes to v2:
- Reshuffle hunks as per suggestion from Johannes

Changes to v3:
- Use GFP_ATOMIC instead of GFP_KERNEL
- Fixup target iteration as reported by Doug Gilbert
- Update scsi_error to make use of the new iterators

Hannes Reinecke (5):
  scsi: convert target lookup to xarray
  target_core_pscsi: use __scsi_device_lookup()
  scsi: move target device list to xarray
  scsi: remove direct device lookup per host
  scsi_error: use xarray lookup instead of wrappers

 drivers/scsi/hosts.c               |   4 +-
 drivers/scsi/scsi.c                | 151 +++++++++++++++++++++++++++++--------
 drivers/scsi/scsi_error.c          |  35 +++++----
 drivers/scsi/scsi_lib.c            |   9 +--
 drivers/scsi/scsi_priv.h           |   2 +
 drivers/scsi/scsi_scan.c           |  73 +++++++++---------
 drivers/scsi/scsi_sysfs.c          |  48 ++++++++----
 drivers/target/target_core_pscsi.c |   8 +-
 include/scsi/scsi_device.h         |  32 +++++---
 include/scsi/scsi_host.h           |   5 +-
 10 files changed, 243 insertions(+), 124 deletions(-)

-- 
2.16.4

[PATCH 1/5] scsi: convert target lookup to xarray

[Hannes Reinecke]

Use an xarray instead of lists for holding the scsi targets.
I've also shortened the 'channel' and 'id' values to 16 bit
as none of the drivers requires a full 32bit range for either
of them, and by shortening them we can use them as the index
into the xarray for storing the scsi_target pointer.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/hosts.c       |  3 ++-
 drivers/scsi/scsi.c        | 32 ++++++++++++++++++------------
 drivers/scsi/scsi_scan.c   | 49 ++++++++++++++++++++--------------------------
 drivers/scsi/scsi_sysfs.c  |  9 ++++++---
 include/scsi/scsi_device.h | 15 +++++++++-----
 include/scsi/scsi_host.h   |  2 +-
 6 files changed, 60 insertions(+), 50 deletions(-)

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 7ec91c3a66ca..87537b0745c1 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -189,6 +189,7 @@ void scsi_remove_host(struct Scsi_Host *shost)
 	transport_unregister_device(&shost->shost_gendev);
 	device_unregister(&shost->shost_dev);
 	device_del(&shost->shost_gendev);
+	xa_destroy(&shost->__targets);
 }
 EXPORT_SYMBOL(scsi_remove_host);
 
@@ -383,7 +384,7 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
 	spin_lock_init(shost->host_lock);
 	shost->shost_state = SHOST_CREATED;
 	INIT_LIST_HEAD(&shost->__devices);
-	INIT_LIST_HEAD(&shost->__targets);
+	xa_init_flags(&shost->__targets, XA_FLAGS_LOCK_IRQ);
 	INIT_LIST_HEAD(&shost->eh_cmd_q);
 	INIT_LIST_HEAD(&shost->starved_list);
 	init_waitqueue_head(&shost->host_wait);
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 56c24a73e0c7..d601424e32b2 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -575,6 +575,19 @@ struct scsi_device *__scsi_iterate_devices(struct Scsi_Host *shost,
 }
 EXPORT_SYMBOL(__scsi_iterate_devices);
 
+/**
+ * __scsi_target_lookup  -  find a target based on channel and target id
+ * @shost:	SCSI host pointer
+ * @channel:	channel number of the target
+ * @id:		ID of the target
+ *
+ */
+static struct scsi_target *__scsi_target_lookup(struct Scsi_Host *shost,
+					 u16 channel, u16 id)
+{
+	return xa_load(&shost->__targets, (channel << 16) | id);
+}
+
 /**
  * starget_for_each_device  -  helper to walk all devices of a target
  * @starget:	target whose devices we want to iterate over.
@@ -701,19 +714,14 @@ EXPORT_SYMBOL(scsi_device_lookup_by_target);
  * really want to use scsi_device_lookup instead.
  **/
 struct scsi_device *__scsi_device_lookup(struct Scsi_Host *shost,
-		uint channel, uint id, u64 lun)
+		u16 channel, u16 id, u64 lun)
 {
-	struct scsi_device *sdev;
+	struct scsi_target *starget;
 
-	list_for_each_entry(sdev, &shost->__devices, siblings) {
-		if (sdev->sdev_state == SDEV_DEL)
-			continue;
-		if (sdev->channel == channel && sdev->id == id &&
-				sdev->lun ==lun)
-			return sdev;
-	}
-
-	return NULL;
+	starget = __scsi_target_lookup(shost, channel, id);
+	if (!starget)
+		return NULL;
+	return __scsi_device_lookup_by_target(starget, lun);
 }
 EXPORT_SYMBOL(__scsi_device_lookup);
 
@@ -729,7 +737,7 @@ EXPORT_SYMBOL(__scsi_device_lookup);
  * needs to be released with scsi_device_put once you're done with it.
  **/
 struct scsi_device *scsi_device_lookup(struct Scsi_Host *shost,
-		uint channel, uint id, u64 lun)
+		u16 channel, u16 id, u64 lun)
 {
 	struct scsi_device *sdev;
 	unsigned long flags;
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index f2437a7570ce..b72265614a9b 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -309,6 +309,7 @@ static void scsi_target_destroy(struct scsi_target *starget)
 	struct device *dev = &starget->dev;
 	struct Scsi_Host *shost = dev_to_shost(dev->parent);
 	unsigned long flags;
+	unsigned long tid = scsi_target_index(starget);
 
 	BUG_ON(starget->state == STARGET_DEL);
 	starget->state = STARGET_DEL;
@@ -316,7 +317,7 @@ static void scsi_target_destroy(struct scsi_target *starget)
 	spin_lock_irqsave(shost->host_lock, flags);
 	if (shost->hostt->target_destroy)
 		shost->hostt->target_destroy(starget);
-	list_del_init(&starget->siblings);
+	xa_erase(&shost->__targets, tid);
 	spin_unlock_irqrestore(shost->host_lock, flags);
 	put_device(dev);
 }
@@ -341,27 +342,6 @@ int scsi_is_target_device(const struct device *dev)
 }
 EXPORT_SYMBOL(scsi_is_target_device);
 
-static struct scsi_target *__scsi_find_target(struct device *parent,
-					      int channel, uint id)
-{
-	struct scsi_target *starget, *found_starget = NULL;
-	struct Scsi_Host *shost = dev_to_shost(parent);
-	/*
-	 * Search for an existing target for this sdev.
-	 */
-	list_for_each_entry(starget, &shost->__targets, siblings) {
-		if (starget->id == id &&
-		    starget->channel == channel) {
-			found_starget = starget;
-			break;
-		}
-	}
-	if (found_starget)
-		get_device(&found_starget->dev);
-
-	return found_starget;
-}
-
 /**
  * scsi_target_reap_ref_release - remove target from visibility
  * @kref: the reap_ref in the target being released
@@ -417,6 +397,7 @@ static struct scsi_target *scsi_alloc_target(struct device *parent,
 	struct scsi_target *starget;
 	struct scsi_target *found_target;
 	int error, ref_got;
+	unsigned long tid;
 
 	starget = kzalloc(size, GFP_KERNEL);
 	if (!starget) {
@@ -433,19 +414,29 @@ static struct scsi_target *scsi_alloc_target(struct device *parent,
 	starget->id = id;
 	starget->channel = channel;
 	starget->can_queue = 0;
-	INIT_LIST_HEAD(&starget->siblings);
 	INIT_LIST_HEAD(&starget->devices);
 	starget->state = STARGET_CREATED;
 	starget->scsi_level = SCSI_2;
 	starget->max_target_blocked = SCSI_DEFAULT_TARGET_BLOCKED;
+	tid = scsi_target_index(starget);
  retry:
 	spin_lock_irqsave(shost->host_lock, flags);
 
-	found_target = __scsi_find_target(parent, channel, id);
-	if (found_target)
+	found_target = xa_load(&shost->__targets, tid);
+	if (found_target) {
+		get_device(&found_target->dev);
 		goto found;
-
-	list_add_tail(&starget->siblings, &shost->__targets);
+	}
+	error = xa_insert(&shost->__targets, tid, starget, GFP_ATOMIC);
+	if (error) {
+		dev_printk(KERN_ERR, dev,
+			     "target %u:%u index allocation failed, error %d\n",
+			     channel, id, error);
+		spin_unlock_irqrestore(shost->host_lock, flags);
+		put_device(dev);
+		kfree(starget);
+		return NULL;
+	}
 	spin_unlock_irqrestore(shost->host_lock, flags);
 	/* allocate and add */
 	transport_setup_device(dev);
@@ -453,7 +444,9 @@ static struct scsi_target *scsi_alloc_target(struct device *parent,
 		error = shost->hostt->target_alloc(starget);
 
 		if(error) {
-			dev_printk(KERN_ERR, dev, "target allocation failed, error %d\n", error);
+			dev_printk(KERN_ERR, dev,
+				   "target %u:%u allocation failed, error %d\n",
+				   channel, id, error);
 			/* don't want scsi_target_reap to do the final
 			 * put because it will be under the host lock */
 			scsi_target_destroy(starget);
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 163dbcb741c1..a694b25be5da 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1512,11 +1512,14 @@ void scsi_remove_target(struct device *dev)
 {
 	struct Scsi_Host *shost = dev_to_shost(dev->parent);
 	struct scsi_target *starget;
+	unsigned long tid = 0;
 	unsigned long flags;
 
-restart:
 	spin_lock_irqsave(shost->host_lock, flags);
-	list_for_each_entry(starget, &shost->__targets, siblings) {
+	for(starget = xa_find(&shost->__targets, &tid, UINT_MAX, XA_PRESENT);
+	    (starget);
+	    starget = xa_find_after(&shost->__targets, &tid,
+				    UINT_MAX, XA_PRESENT)) {
 		if (starget->state == STARGET_DEL ||
 		    starget->state == STARGET_REMOVE ||
 		    starget->state == STARGET_CREATED_REMOVE)
@@ -1530,7 +1533,7 @@ void scsi_remove_target(struct device *dev)
 			spin_unlock_irqrestore(shost->host_lock, flags);
 			__scsi_remove_target(starget);
 			scsi_target_reap(starget);
-			goto restart;
+			spin_lock_irqsave(shost->host_lock, flags);
 		}
 	}
 	spin_unlock_irqrestore(shost->host_lock, flags);
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index c3cba2aaf934..8cb31bbd8a82 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -121,7 +121,7 @@ struct scsi_device {
 
 	unsigned long last_queue_ramp_up;	/* last queue ramp up time */
 
-	unsigned int id, channel;
+	u16 id, channel;
 	u64 lun;
 	unsigned int manufacturer;	/* Manufacturer of device, for using 
 					 * vendor-specific cmd's */
@@ -288,8 +288,8 @@ struct scsi_target {
 	struct list_head	devices;
 	struct device		dev;
 	struct kref		reap_ref; /* last put renders target invisible */
-	unsigned int		channel;
-	unsigned int		id; /* target id ... replace
+	u16			channel;
+	u16			id; /* target id ... replace
 				     * scsi_device.id eventually */
 	unsigned int		create:1; /* signal that it needs to be added */
 	unsigned int		single_lun:1;	/* Indicates we should only
@@ -321,6 +321,11 @@ struct scsi_target {
 	/* starget_data must be the last element!!!! */
 } __attribute__((aligned(sizeof(unsigned long))));
 
+static inline u32 scsi_target_index(struct scsi_target *starget)
+{
+	return (starget->channel << 16) | (starget->id);
+}
+
 #define to_scsi_target(d)	container_of(d, struct scsi_target, dev)
 static inline struct scsi_target *scsi_target(struct scsi_device *sdev)
 {
@@ -345,9 +350,9 @@ extern struct scsi_device *scsi_device_from_queue(struct request_queue *q);
 extern int __must_check scsi_device_get(struct scsi_device *);
 extern void scsi_device_put(struct scsi_device *);
 extern struct scsi_device *scsi_device_lookup(struct Scsi_Host *,
-					      uint, uint, u64);
+					      u16, u16, u64);
 extern struct scsi_device *__scsi_device_lookup(struct Scsi_Host *,
-						uint, uint, u64);
+						u16, u16, u64);
 extern struct scsi_device *scsi_device_lookup_by_target(struct scsi_target *,
 							u64);
 extern struct scsi_device *__scsi_device_lookup_by_target(struct scsi_target *,
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 822e8cda8d9b..b9395676c75b 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -521,7 +521,7 @@ struct Scsi_Host {
 	 * access this list directly from a driver.
 	 */
 	struct list_head	__devices;
-	struct list_head	__targets;
+	struct xarray		__targets;
 	
 	struct list_head	starved_list;
 
-- 
2.16.4

[PATCH 2/5] target_core_pscsi: use __scsi_device_lookup()

[Hannes Reinecke]

Instead of walking the list of devices manually use the helper
function to return the device directly.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: Bart van Assche <bvanassche@acm.org>
---
 drivers/target/target_core_pscsi.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c
index c9d92b3e777d..5fb852d1281f 100644
--- a/drivers/target/target_core_pscsi.c
+++ b/drivers/target/target_core_pscsi.c
@@ -496,11 +496,9 @@ static int pscsi_configure_device(struct se_device *dev)
 	}
 
 	spin_lock_irq(sh->host_lock);
-	list_for_each_entry(sd, &sh->__devices, siblings) {
-		if ((pdv->pdv_channel_id != sd->channel) ||
-		    (pdv->pdv_target_id != sd->id) ||
-		    (pdv->pdv_lun_id != sd->lun))
-			continue;
+	sd = __scsi_device_lookup(sh, pdv->pdv_channel_id,
+				  pdv->pdv_target_id, pdv->pdv_lun_id);
+	if (sd) {
 		/*
 		 * Functions will release the held struct scsi_host->host_lock
 		 * before calling calling pscsi_add_device_to_list() to register
-- 
2.16.4

[PATCH 3/5] scsi: move target device list to xarray

[Hannes Reinecke]

Use xarray for device lookup by target. LUNs below 256 are linear,
and can be used directly as the index into the xarray.
LUNs above 256 have a distinct LUN format, and are not necessarily
linear. They'll be stored in indices above 256 in the xarray, with
the next free index in the xarray.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 drivers/scsi/scsi.c        | 35 +++++++++++++++++++++--------------
 drivers/scsi/scsi_lib.c    |  9 ++++-----
 drivers/scsi/scsi_scan.c   |  4 ++--
 drivers/scsi/scsi_sysfs.c  | 29 +++++++++++++++++++++++++++--
 include/scsi/scsi_device.h |  4 ++--
 5 files changed, 56 insertions(+), 25 deletions(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index d601424e32b2..a3e01708744f 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -601,13 +601,14 @@ static struct scsi_target *__scsi_target_lookup(struct Scsi_Host *shost,
 void starget_for_each_device(struct scsi_target *starget, void *data,
 		     void (*fn)(struct scsi_device *, void *))
 {
-	struct Scsi_Host *shost = dev_to_shost(starget->dev.parent);
 	struct scsi_device *sdev;
+	unsigned long lun_idx = 0;
 
-	shost_for_each_device(sdev, shost) {
-		if ((sdev->channel == starget->channel) &&
-		    (sdev->id == starget->id))
-			fn(sdev, data);
+	xa_for_each(&starget->__devices, lun_idx, sdev) {
+		if (scsi_device_get(sdev))
+			continue;
+		fn(sdev, data);
+		scsi_device_put(sdev);
 	}
 }
 EXPORT_SYMBOL(starget_for_each_device);
@@ -629,14 +630,11 @@ EXPORT_SYMBOL(starget_for_each_device);
 void __starget_for_each_device(struct scsi_target *starget, void *data,
 			       void (*fn)(struct scsi_device *, void *))
 {
-	struct Scsi_Host *shost = dev_to_shost(starget->dev.parent);
 	struct scsi_device *sdev;
+	unsigned long lun_idx = 0;
 
-	__shost_for_each_device(sdev, shost) {
-		if ((sdev->channel == starget->channel) &&
-		    (sdev->id == starget->id))
-			fn(sdev, data);
-	}
+	xa_for_each(&starget->__devices, lun_idx, sdev)
+		fn(sdev, data);
 }
 EXPORT_SYMBOL(__starget_for_each_device);
 
@@ -658,12 +656,21 @@ EXPORT_SYMBOL(__starget_for_each_device);
 struct scsi_device *__scsi_device_lookup_by_target(struct scsi_target *starget,
 						   u64 lun)
 {
-	struct scsi_device *sdev;
+	struct scsi_device *sdev = NULL;
+	unsigned long lun_idx = 0;
+
+	if (lun < UINT_MAX)
+		sdev = xa_load(&starget->__devices, lun);
+	if (sdev && sdev->lun == lun) {
+		if (sdev->sdev_state == SDEV_DEL)
+			sdev = NULL;
+		return sdev;
+	}
 
-	list_for_each_entry(sdev, &starget->devices, same_target_siblings) {
+	xa_for_each(&starget->__devices, lun_idx, sdev) {
 		if (sdev->sdev_state == SDEV_DEL)
 			continue;
-		if (sdev->lun ==lun)
+		if (sdev->lun == lun)
 			return sdev;
 	}
 
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 06c260f6cdae..fc01591d32a8 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -372,9 +372,9 @@ static void scsi_kick_queue(struct request_queue *q)
 static void scsi_single_lun_run(struct scsi_device *current_sdev)
 {
 	struct Scsi_Host *shost = current_sdev->host;
-	struct scsi_device *sdev, *tmp;
+	struct scsi_device *sdev;
 	struct scsi_target *starget = scsi_target(current_sdev);
-	unsigned long flags;
+	unsigned long flags, lun_idx = 0;
 
 	spin_lock_irqsave(shost->host_lock, flags);
 	starget->starget_sdev_user = NULL;
@@ -391,8 +391,7 @@ static void scsi_single_lun_run(struct scsi_device *current_sdev)
 	spin_lock_irqsave(shost->host_lock, flags);
 	if (starget->starget_sdev_user)
 		goto out;
-	list_for_each_entry_safe(sdev, tmp, &starget->devices,
-			same_target_siblings) {
+	xa_for_each(&starget->__devices, lun_idx, sdev) {
 		if (sdev == current_sdev)
 			continue;
 		if (scsi_device_get(sdev))
@@ -401,7 +400,7 @@ static void scsi_single_lun_run(struct scsi_device *current_sdev)
 		spin_unlock_irqrestore(shost->host_lock, flags);
 		scsi_kick_queue(sdev->request_queue);
 		spin_lock_irqsave(shost->host_lock, flags);
-	
+
 		scsi_device_put(sdev);
 	}
  out:
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index b72265614a9b..e88ee895ab2a 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -235,7 +235,6 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget,
 	mutex_init(&sdev->state_mutex);
 	sdev->sdev_state = SDEV_CREATED;
 	INIT_LIST_HEAD(&sdev->siblings);
-	INIT_LIST_HEAD(&sdev->same_target_siblings);
 	INIT_LIST_HEAD(&sdev->starved_entry);
 	INIT_LIST_HEAD(&sdev->event_list);
 	spin_lock_init(&sdev->list_lock);
@@ -327,6 +326,7 @@ static void scsi_target_dev_release(struct device *dev)
 	struct device *parent = dev->parent;
 	struct scsi_target *starget = to_scsi_target(dev);
 
+	xa_destroy(&starget->__devices);
 	kfree(starget);
 	put_device(parent);
 }
@@ -414,7 +414,7 @@ static struct scsi_target *scsi_alloc_target(struct device *parent,
 	starget->id = id;
 	starget->channel = channel;
 	starget->can_queue = 0;
-	INIT_LIST_HEAD(&starget->devices);
+	xa_init_flags(&starget->__devices, XA_FLAGS_ALLOC);
 	starget->state = STARGET_CREATED;
 	starget->scsi_level = SCSI_2;
 	starget->max_target_blocked = SCSI_DEFAULT_TARGET_BLOCKED;
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index a694b25be5da..05a0111d4a6d 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -434,6 +434,7 @@ static void scsi_device_cls_release(struct device *class_dev)
 static void scsi_device_dev_release_usercontext(struct work_struct *work)
 {
 	struct scsi_device *sdev;
+	struct scsi_target *starget;
 	struct device *parent;
 	struct list_head *this, *tmp;
 	struct scsi_vpd *vpd_pg80 = NULL, *vpd_pg83 = NULL;
@@ -441,6 +442,7 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work)
 	unsigned long flags;
 
 	sdev = container_of(work, struct scsi_device, ew.work);
+	starget = sdev->sdev_target;
 
 	scsi_dh_release_device(sdev);
 
@@ -448,7 +450,7 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work)
 
 	spin_lock_irqsave(sdev->host->host_lock, flags);
 	list_del(&sdev->siblings);
-	list_del(&sdev->same_target_siblings);
+	xa_erase(&starget->__devices, sdev->lun_idx);
 	list_del(&sdev->starved_entry);
 	spin_unlock_irqrestore(sdev->host->host_lock, flags);
 
@@ -1590,6 +1592,7 @@ void scsi_sysfs_device_initialize(struct scsi_device *sdev)
 	unsigned long flags;
 	struct Scsi_Host *shost = sdev->host;
 	struct scsi_target  *starget = sdev->sdev_target;
+	int ret = -ERANGE;
 
 	device_initialize(&sdev->sdev_gendev);
 	sdev->sdev_gendev.bus = &scsi_bus_type;
@@ -1617,7 +1620,29 @@ void scsi_sysfs_device_initialize(struct scsi_device *sdev)
 
 	transport_setup_device(&sdev->sdev_gendev);
 	spin_lock_irqsave(shost->host_lock, flags);
-	list_add_tail(&sdev->same_target_siblings, &starget->devices);
+	if (sdev->lun < 256) {
+		sdev->lun_idx = sdev->lun;
+		ret = xa_insert(&starget->__devices, (unsigned int)sdev->lun,
+				sdev, GFP_ATOMIC);
+	}
+	/*
+	 * Use a free index if the LUN is larger than 32 bit or
+	 * if the index is already taken. Leave the lower 256
+	 * entries free to avoid collisions.
+	 */
+	if (ret) {
+		struct xa_limit scsi_lun_limit = {
+			.min = 256,
+			.max = UINT_MAX,
+		};
+		ret = xa_alloc(&starget->__devices, &sdev->lun_idx,
+			       sdev, scsi_lun_limit, GFP_ATOMIC);
+	}
+	if (ret) {
+		shost_printk(KERN_ERR, shost,
+			     "LUN index %u:%u:%llu allocation error %d\n",
+			     sdev_channel(sdev), sdev_id(sdev), sdev->lun, ret);
+	}
 	list_add_tail(&sdev->siblings, &shost->__devices);
 	spin_unlock_irqrestore(shost->host_lock, flags);
 	/*
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 8cb31bbd8a82..56801661f481 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -104,7 +104,6 @@ struct scsi_device {
 
 	/* the next two are protected by the host->host_lock */
 	struct list_head    siblings;   /* list of all devices on this host */
-	struct list_head    same_target_siblings; /* just the devices sharing same target id */
 
 	atomic_t device_busy;		/* commands actually active on LLDD */
 	atomic_t device_blocked;	/* Device returned QUEUE_FULL. */
@@ -123,6 +122,7 @@ struct scsi_device {
 
 	u16 id, channel;
 	u64 lun;
+	unsigned int lun_idx;		/* Index into target device xarray */
 	unsigned int manufacturer;	/* Manufacturer of device, for using 
 					 * vendor-specific cmd's */
 	unsigned sector_size;	/* size in bytes */
@@ -285,7 +285,7 @@ enum scsi_target_state {
 struct scsi_target {
 	struct scsi_device	*starget_sdev_user;
 	struct list_head	siblings;
-	struct list_head	devices;
+	struct xarray		__devices;
 	struct device		dev;
 	struct kref		reap_ref; /* last put renders target invisible */
 	u16			channel;
-- 
2.16.4

[PATCH 4/5] scsi: remove direct device lookup per host

[Hannes Reinecke]

Drop the per-host device list for direct lookup and iterate
over the targets and devices xarrays instead.
As both are now using xarrays the lookup is more efficient
as we can use the provided indices based on the HCTL id
to do a direct lookup instead of traversing lists.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/hosts.c       |  1 -
 drivers/scsi/scsi.c        | 84 ++++++++++++++++++++++++++++++++++++++++++----
 drivers/scsi/scsi_scan.c   | 20 ++++++-----
 drivers/scsi/scsi_sysfs.c  | 10 ++----
 include/scsi/scsi_device.h | 13 ++++---
 include/scsi/scsi_host.h   |  3 +-
 6 files changed, 100 insertions(+), 31 deletions(-)

diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 87537b0745c1..8cd7cd192bae 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -383,7 +383,6 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize)
 	shost->host_lock = &shost->default_lock;
 	spin_lock_init(shost->host_lock);
 	shost->shost_state = SHOST_CREATED;
-	INIT_LIST_HEAD(&shost->__devices);
 	xa_init_flags(&shost->__targets, XA_FLAGS_LOCK_IRQ);
 	INIT_LIST_HEAD(&shost->eh_cmd_q);
 	INIT_LIST_HEAD(&shost->starved_list);
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index a3e01708744f..6a1d8c6bd8f9 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -554,18 +554,48 @@ EXPORT_SYMBOL(scsi_device_put);
 struct scsi_device *__scsi_iterate_devices(struct Scsi_Host *shost,
 					   struct scsi_device *prev)
 {
-	struct list_head *list = (prev ? &prev->siblings : &shost->__devices);
+	struct scsi_target *starget;
 	struct scsi_device *next = NULL;
 	unsigned long flags;
+	unsigned long tid = 0, lun_idx = 0;
 
 	spin_lock_irqsave(shost->host_lock, flags);
-	while (list->next != &shost->__devices) {
-		next = list_entry(list->next, struct scsi_device, siblings);
-		/* skip devices that we can't get a reference to */
-		if (!scsi_device_get(next))
+	if (!prev) {
+		starget = xa_find(&shost->__targets, &tid,
+				       UINT_MAX, XA_PRESENT);
+		if (starget) {
+			next = xa_find(&starget->__devices, &lun_idx,
+				       UINT_MAX, XA_PRESENT);
+			if (!scsi_device_get(next)) {
+				spin_unlock_irqrestore(shost->host_lock, flags);
+				return next;
+			}
+		}
+	} else {
+		starget = prev->sdev_target;
+		tid = (starget->channel << 16) | starget->id;
+		lun_idx = prev->lun_idx;
+	}
+	while (starget) {
+		next = xa_find_after(&starget->__devices, &lun_idx,
+				     UINT_MAX, XA_PRESENT);
+		if (next) {
+			if (!scsi_device_get(next))
+				break;
+			continue;
+		}
+		/* No more LUNs on this target, switch to the next */
+		starget = xa_find_after(&shost->__targets, &tid,
+					UINT_MAX, XA_PRESENT);
+		if (!starget) {
+			next = NULL;
+			break;
+		}
+		lun_idx = 0;
+		next = xa_find(&starget->__devices, &lun_idx,
+			       UINT_MAX, XA_PRESENT);
+		if (next && !scsi_device_get(next))
 			break;
-		next = NULL;
-		list = list->next;
 	}
 	spin_unlock_irqrestore(shost->host_lock, flags);
 
@@ -575,6 +605,46 @@ struct scsi_device *__scsi_iterate_devices(struct Scsi_Host *shost,
 }
 EXPORT_SYMBOL(__scsi_iterate_devices);
 
+/* helper for __shost_for_each_device, see that for documentation */
+struct scsi_device *__scsi_iterate_devices_unlocked(struct Scsi_Host *shost,
+						    struct scsi_device *prev)
+{
+	struct scsi_target *starget;
+	struct scsi_device *next = NULL;
+	unsigned long tid = 0, lun_idx = 0;
+
+	if (!prev) {
+		starget = xa_find(&shost->__targets, &tid,
+				       UINT_MAX, XA_PRESENT);
+		if (starget)
+			return xa_find(&starget->__devices, &lun_idx,
+				       UINT_MAX, XA_PRESENT);
+	} else {
+		starget = prev->sdev_target;
+		tid = scsi_target_index(starget);
+		lun_idx = prev->lun_idx;
+	}
+	while (starget) {
+		next = xa_find_after(&starget->__devices, &lun_idx,
+				     UINT_MAX, XA_PRESENT);
+		if (next)
+			return next;
+		/* No more LUNs on this target, switch to the next */
+		starget = xa_find_after(&shost->__targets, &tid,
+					UINT_MAX, XA_PRESENT);
+		if (!starget)
+			return NULL;
+
+		lun_idx = 0;
+		next = xa_find(&starget->__devices, &lun_idx,
+			       UINT_MAX, XA_PRESENT);
+		if (next)
+			return next;
+	}
+	return NULL;
+}
+EXPORT_SYMBOL(__scsi_iterate_devices_unlocked);
+
 /**
  * __scsi_target_lookup  -  find a target based on channel and target id
  * @shost:	SCSI host pointer
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index e88ee895ab2a..82a00d7751b3 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -234,7 +234,6 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget,
 	sdev->channel = starget->channel;
 	mutex_init(&sdev->state_mutex);
 	sdev->sdev_state = SDEV_CREATED;
-	INIT_LIST_HEAD(&sdev->siblings);
 	INIT_LIST_HEAD(&sdev->starved_entry);
 	INIT_LIST_HEAD(&sdev->event_list);
 	spin_lock_init(&sdev->list_lock);
@@ -1852,17 +1851,22 @@ EXPORT_SYMBOL(scsi_scan_host);
 
 void scsi_forget_host(struct Scsi_Host *shost)
 {
+	struct scsi_target *starget;
 	struct scsi_device *sdev;
 	unsigned long flags;
+	unsigned long tid = 0;
 
- restart:
 	spin_lock_irqsave(shost->host_lock, flags);
-	list_for_each_entry(sdev, &shost->__devices, siblings) {
-		if (sdev->sdev_state == SDEV_DEL)
-			continue;
-		spin_unlock_irqrestore(shost->host_lock, flags);
-		__scsi_remove_device(sdev);
-		goto restart;
+	xa_for_each(&shost->__targets, tid, starget) {
+		unsigned long lun_id = 0;
+
+		xa_for_each(&starget->__devices, lun_id, sdev) {
+			if (sdev->sdev_state == SDEV_DEL)
+				continue;
+			spin_unlock_irqrestore(shost->host_lock, flags);
+			__scsi_remove_device(sdev);
+			spin_lock_irqsave(shost->host_lock, flags);
+		}
 	}
 	spin_unlock_irqrestore(shost->host_lock, flags);
 }
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 05a0111d4a6d..4d40801a3961 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -449,7 +449,6 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work)
 	parent = sdev->sdev_gendev.parent;
 
 	spin_lock_irqsave(sdev->host->host_lock, flags);
-	list_del(&sdev->siblings);
 	xa_erase(&starget->__devices, sdev->lun_idx);
 	list_del(&sdev->starved_entry);
 	spin_unlock_irqrestore(sdev->host->host_lock, flags);
@@ -1476,19 +1475,16 @@ static void __scsi_remove_target(struct scsi_target *starget)
 	struct Scsi_Host *shost = dev_to_shost(starget->dev.parent);
 	unsigned long flags;
 	struct scsi_device *sdev;
+	unsigned long lun_idx = 0;
 
 	spin_lock_irqsave(shost->host_lock, flags);
- restart:
-	list_for_each_entry(sdev, &shost->__devices, siblings) {
+	xa_for_each(&starget->__devices, lun_idx, sdev) {
 		/*
 		 * We cannot call scsi_device_get() here, as
 		 * we might've been called from rmmod() causing
 		 * scsi_device_get() to fail the module_is_live()
 		 * check.
 		 */
-		if (sdev->channel != starget->channel ||
-		    sdev->id != starget->id)
-			continue;
 		if (sdev->sdev_state == SDEV_DEL ||
 		    sdev->sdev_state == SDEV_CANCEL ||
 		    !get_device(&sdev->sdev_gendev))
@@ -1497,7 +1493,6 @@ static void __scsi_remove_target(struct scsi_target *starget)
 		scsi_remove_device(sdev);
 		put_device(&sdev->sdev_gendev);
 		spin_lock_irqsave(shost->host_lock, flags);
-		goto restart;
 	}
 	spin_unlock_irqrestore(shost->host_lock, flags);
 }
@@ -1643,7 +1638,6 @@ void scsi_sysfs_device_initialize(struct scsi_device *sdev)
 			     "LUN index %u:%u:%llu allocation error %d\n",
 			     sdev_channel(sdev), sdev_id(sdev), sdev->lun, ret);
 	}
-	list_add_tail(&sdev->siblings, &shost->__devices);
 	spin_unlock_irqrestore(shost->host_lock, flags);
 	/*
 	 * device can now only be removed via __scsi_remove_device() so hold
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index 56801661f481..3690af6ce6af 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -102,9 +102,6 @@ struct scsi_device {
 	struct Scsi_Host *host;
 	struct request_queue *request_queue;
 
-	/* the next two are protected by the host->host_lock */
-	struct list_head    siblings;   /* list of all devices on this host */
-
 	atomic_t device_busy;		/* commands actually active on LLDD */
 	atomic_t device_blocked;	/* Device returned QUEUE_FULL. */
 
@@ -381,6 +378,10 @@ extern struct scsi_device *__scsi_iterate_devices(struct Scsi_Host *,
 	     (sdev); \
 	     (sdev) = __scsi_iterate_devices((shost), (sdev)))
 
+/* only exposed to implement shost_for_each_device */
+struct scsi_device *__scsi_iterate_devices_unlocked(struct Scsi_Host *,
+						    struct scsi_device *);
+
 /**
  * __shost_for_each_device - iterate over all devices of a host (UNLOCKED)
  * @sdev: the &struct scsi_device to use as a cursor
@@ -394,8 +395,10 @@ extern struct scsi_device *__scsi_iterate_devices(struct Scsi_Host *,
  * device list in interrupt context.  Otherwise you really want to use
  * shost_for_each_device instead.
  */
-#define __shost_for_each_device(sdev, shost) \
-	list_for_each_entry((sdev), &((shost)->__devices), siblings)
+#define __shost_for_each_device(sdev, shost)				\
+	for((sdev) = __scsi_iterate_devices_unlocked((shost), NULL);	\
+	    (sdev);							\
+	    (sdev) = __scsi_iterate_devices_unlocked((shost),(sdev)))
 
 extern int scsi_change_queue_depth(struct scsi_device *, int);
 extern int scsi_track_queue_full(struct scsi_device *, int);
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index b9395676c75b..ee0b72075e9f 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -520,9 +520,8 @@ struct Scsi_Host {
 	 * their __ prefixed variants with the lock held. NEVER
 	 * access this list directly from a driver.
 	 */
-	struct list_head	__devices;
 	struct xarray		__targets;
-	
+
 	struct list_head	starved_list;
 
 	spinlock_t		default_lock;
-- 
2.16.4

[PATCH 5/5] scsi_error: use xarray lookup instead of wrappers

[Hannes Reinecke]

For SCSI EH most shost_for_each_sdev() calls are just to filter
out devices for specific targets or channels.
These calls can be made more efficient using direct xarray
lookup and iterators.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/scsi/scsi.c       |  2 +-
 drivers/scsi/scsi_error.c | 35 ++++++++++++++++++++++-------------
 drivers/scsi/scsi_priv.h  |  2 ++
 3 files changed, 25 insertions(+), 14 deletions(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 6a1d8c6bd8f9..296cecd61d3a 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -652,7 +652,7 @@ EXPORT_SYMBOL(__scsi_iterate_devices_unlocked);
  * @id:		ID of the target
  *
  */
-static struct scsi_target *__scsi_target_lookup(struct Scsi_Host *shost,
+struct scsi_target *__scsi_target_lookup(struct Scsi_Host *shost,
 					 u16 channel, u16 id)
 {
 	return xa_load(&shost->__targets, (channel << 16) | id);
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 978be1602f71..f13789e86601 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -643,7 +643,9 @@ EXPORT_SYMBOL_GPL(scsi_check_sense);
 static void scsi_handle_queue_ramp_up(struct scsi_device *sdev)
 {
 	struct scsi_host_template *sht = sdev->host->hostt;
+	struct scsi_target *starget;
 	struct scsi_device *tmp_sdev;
+	unsigned long lun_idx = 0;
 
 	if (!sht->track_queue_depth ||
 	    sdev->queue_depth >= sdev->max_queue_depth)
@@ -661,10 +663,9 @@ static void scsi_handle_queue_ramp_up(struct scsi_device *sdev)
 	 * Walk all devices of a target and do
 	 * ramp up on them.
 	 */
-	shost_for_each_device(tmp_sdev, sdev->host) {
-		if (tmp_sdev->channel != sdev->channel ||
-		    tmp_sdev->id != sdev->id ||
-		    tmp_sdev->queue_depth == sdev->max_queue_depth)
+	starget = __scsi_target_lookup(sdev->host, sdev->channel, sdev->id);
+	xa_for_each(&starget->__devices, lun_idx, tmp_sdev) {
+		if (tmp_sdev->queue_depth == sdev->max_queue_depth)
 			continue;
 
 		scsi_change_queue_depth(tmp_sdev, tmp_sdev->queue_depth + 1);
@@ -675,14 +676,15 @@ static void scsi_handle_queue_ramp_up(struct scsi_device *sdev)
 static void scsi_handle_queue_full(struct scsi_device *sdev)
 {
 	struct scsi_host_template *sht = sdev->host->hostt;
+	struct scsi_target *starget;
 	struct scsi_device *tmp_sdev;
+	unsigned long lun_idx = 0;
 
 	if (!sht->track_queue_depth)
 		return;
 
-	shost_for_each_device(tmp_sdev, sdev->host) {
-		if (tmp_sdev->channel != sdev->channel ||
-		    tmp_sdev->id != sdev->id)
+	xa_for_each(&starget->__devices, lun_idx, tmp_sdev) {
+		if (tmp_sdev->sdev_state == SDEV_DEL)
 			continue;
 		/*
 		 * We do not know the number of commands that were at
@@ -2271,10 +2273,16 @@ int scsi_error_handler(void *data)
  */
 void scsi_report_bus_reset(struct Scsi_Host *shost, int channel)
 {
+	struct scsi_target *starget;
 	struct scsi_device *sdev;
+	unsigned long tid = 0;
 
-	__shost_for_each_device(sdev, shost) {
-		if (channel == sdev_channel(sdev))
+	xa_for_each(&shost->__targets, tid, starget) {
+		unsigned long lun_idx = 0;
+
+		if (starget->channel != channel)
+			continue;
+		xa_for_each(&starget->__devices, lun_idx, sdev)
 			__scsi_report_device_reset(sdev, NULL);
 	}
 }
@@ -2304,13 +2312,14 @@ EXPORT_SYMBOL(scsi_report_bus_reset);
  */
 void scsi_report_device_reset(struct Scsi_Host *shost, int channel, int target)
 {
+	struct scsi_target *starget;
 	struct scsi_device *sdev;
+	unsigned long lun_idx = 0;
 
-	__shost_for_each_device(sdev, shost) {
-		if (channel == sdev_channel(sdev) &&
-		    target == sdev_id(sdev))
+	starget = __scsi_target_lookup(shost, channel, target);
+	if (starget)
+		xa_for_each(&starget->__devices, lun_idx, sdev)
 			__scsi_report_device_reset(sdev, NULL);
-	}
 }
 EXPORT_SYMBOL(scsi_report_device_reset);
 
diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index 22b6585e28b4..0a87c95359aa 100644
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -49,6 +49,8 @@ enum scsi_devinfo_key {
 	SCSI_DEVINFO_SPI,
 };
 
+extern struct scsi_target *__scsi_target_lookup(struct Scsi_Host *shost,
+						u16 channel, u16 id);
 extern blist_flags_t scsi_get_device_flags(struct scsi_device *sdev,
 					   const unsigned char *vendor,
 					   const unsigned char *model);
-- 
2.16.4

Re: [PATCHv4 0/5] scsi: use xarray for devices and targets

[Douglas Gilbert]

[-- Attachment #1: Type: text/plain, Size: 8028 bytes --]

On 2020-05-29 9:47 a.m., Hannes Reinecke wrote:
> Hi all,
> 
> based on the ideas from Doug Gilbert here's now my take on using
> xarrays for devices and targets.
> It revolves around two ideas:
> 
>   - The scsi target 'channel' and 'id' numbers are never ever used
>     to the full 32 bit range; channels are well below 10, and no
>     driver is using more than 16 bits for the id. So we can reduce
>     the type of 'channel' and 'id' to 16 bits, and use the 32 bit
>     value 'channel << 16 | id' as the index into the target xarray.
>   - Most SCSI targets are only using the first 32 bits of the 64 bit
>     LUN structure. So there we can use the LUN number as the index into
>     the xarray; larger LUN numbers won't fit, so we'll allocate a
>     separate index for those. For these device lookup won't be as
>     efficient, but one can argue that we're running into scalability
>     issues long before that.
> 
> With these changes we can implement an efficient lookup mechanism,
> devolving into direct lookup for most cases.
> And iteration over devices should be as efficient as the current,
> list-based, approach.
> 
> It also removes the current duality between host-based and
> target-based device lists; now all devices are listed per target,
> and a full iteration would need to iterate first over all targets
> and then over all devices per target.
> 
> As usual, comments and reviews are welcome
> 
> Changes to v1:
> - Fixup __scsi_iterate_devices()
> - Include reviews from Johannes
> - Minor fixes
> - Include comments from Doug
> 
> Changes to v2:
> - Reshuffle hunks as per suggestion from Johannes
> 
> Changes to v3:
> - Use GFP_ATOMIC instead of GFP_KERNEL
> - Fixup target iteration as reported by Doug Gilbert
> - Update scsi_error to make use of the new iterators
> 
> Hannes Reinecke (5):
>    scsi: convert target lookup to xarray
>    target_core_pscsi: use __scsi_device_lookup()
>    scsi: move target device list to xarray
>    scsi: remove direct device lookup per host
>    scsi_error: use xarray lookup instead of wrappers
> 
>   drivers/scsi/hosts.c               |   4 +-
>   drivers/scsi/scsi.c                | 151 +++++++++++++++++++++++++++++--------
>   drivers/scsi/scsi_error.c          |  35 +++++----
>   drivers/scsi/scsi_lib.c            |   9 +--
>   drivers/scsi/scsi_priv.h           |   2 +
>   drivers/scsi/scsi_scan.c           |  73 +++++++++---------
>   drivers/scsi/scsi_sysfs.c          |  48 ++++++++----
>   drivers/target/target_core_pscsi.c |   8 +-
>   include/scsi/scsi_device.h         |  32 +++++---
>   include/scsi/scsi_host.h           |   5 +-
>   10 files changed, 243 insertions(+), 124 deletions(-)
> 

Hannes,
I'm pretty sure this one was happening yesterday, around about the same
number of sdev_s (i.e. sg791). The missing 0:0:1:0, 0:0:2:0, etc
distracted me. That is now fixed. See attached.

It was executing an rmmod at the time in the tst_sdebug_modpr_rmmod.sh
script. Just checked my logs from yesterday. It blew up at the same spot
in the code (but after /dev/sg971):

...
2020-05-28T20:44:31.543998-04:00 xtwo70 kernel: scsi 11:0:8:8: Attached scsi 
generic sg969 type 9
2020-05-28T20:44:31.545999-04:00 xtwo70 kernel: scsi 11:0:8:9: Attached scsi 
generic sg970 type 9
2020-05-28T20:44:31.558008-04:00 xtwo70 kernel: scsi 11:0:8:10: Attached scsi 
generic sg971 type 9
2020-05-28T20:44:45.660126-04:00 xtwo70 kernel: [UFW BLOCK] IN=enp0s31f6 OUT= 
MAC=54:e1:ad:36:d8:55:00:15:99:30:1d:2b:08:00 SRC=192.168.48.94 
DST=192.168.48.23 LEN=224 TOS=0x00 PREC=0x00 TTL=64 ID=34650 PROTO=UDP SPT=161 
DPT=48409 LEN=204
2020-05-28T20:44:45.692021-04:00 xtwo70 kernel: [UFW BLOCK] IN=enp0s31f6 OUT= 
MAC=54:e1:ad:36:d8:55:00:15:99:e9:97:99:08:00 SRC=192.168.48.55 
DST=192.168.48.23 LEN=258 TOS=0x00 PREC=0x00 TTL=64 ID=60034 PROTO=UDP SPT=161 
DPT=48409 LEN=238
2020-05-28T20:44:46.660191-04:00 xtwo70 kernel: [UFW BLOCK] IN=enp0s31f6 OUT= 
MAC=54:e1:ad:36:d8:55:00:15:99:e9:97:99:08:00 SRC=192.168.48.55 
DST=192.168.48.23 LEN=258 TOS=0x00 PREC=0x00 TTL=64 ID=60290 PROTO=UDP SPT=161 
DPT=48409 LEN=238
2020-05-28T20:44:46.660273-04:00 xtwo70 kernel: [UFW BLOCK] IN=enp0s31f6 OUT= 
MAC=54:e1:ad:36:d8:55:00:15:99:30:1d:2b:08:00 SRC=192.168.48.94 
DST=192.168.48.23 LEN=224 TOS=0x00 PREC=0x00 TTL=64 ID=34651 PROTO=UDP SPT=161 
DPT=48409 LEN=204
2020-05-28T20:46:46.306581-04:00 xtwo70 systemd-resolved[1953]: Server returned 
error NXDOMAIN, mitigating potential DNS violation DVE-2018-0001, retrying 
transaction with reduced feature level UDP.
2020-05-28T20:47:05.776485-04:00 xtwo70 kernel: BUG: unable to handle page fault 
for address: ffff8881327c6868
2020-05-28T20:47:05.776504-04:00 xtwo70 kernel: #PF: supervisor read access in 
kernel mode
2020-05-28T20:47:05.776506-04:00 xtwo70 kernel: #PF: error_code(0x0000) - 
not-present page
2020-05-28T20:47:05.776507-04:00 xtwo70 kernel: PGD 3c01067 P4D 3c01067 PUD 
22f031067 PMD 22ee9d067 PTE 800ffffecd839060
2020-05-28T20:47:05.776509-04:00 xtwo70 kernel: Oops: 0000 [#1] PREEMPT SMP 
DEBUG_PAGEALLOC PTI
2020-05-28T20:47:05.776510-04:00 xtwo70 kernel: CPU: 2 PID: 4647 Comm: rmmod Not 
tainted 5.7.0-rc1+ #74
2020-05-28T20:47:05.776511-04:00 xtwo70 kernel: Hardware name: LENOVO 
20K5S27Q02/20K5S27Q02, BIOS R0IET60W (1.38 ) 12/19/2019
2020-05-28T20:47:05.776512-04:00 xtwo70 kernel: RIP: 0010:xas_start+0x2a/0x1c0
2020-05-28T20:47:05.776513-04:00 xtwo70 kernel: Code: 41 55 41 54 55 53 48 8b 47 
18 48 89 fb 48 89 c2 83 e2 03 0f 84 8d 00 00 00 48 3d 05 c0 ff ff 76 06 48 83 fa 
02 74 50 4c 8b 2b <49> 8b 6d 50 e8 fd 2f c2 ff 85 c0 49 89 ec 74 0d 80 3d ef f0 
e4 00
2020-05-28T20:47:05.776514-04:00 xtwo70 kernel: RSP: 0000:ffffc900005bbc90 
EFLAGS: 00010013
2020-05-28T20:47:05.776514-04:00 xtwo70 kernel: RAX: 0000000000000003 RBX: 
ffffc900005bbd00 RCX: 00000000ceb8764a
2020-05-28T20:47:05.776515-04:00 xtwo70 kernel: RDX: 0000000000000003 RSI: 
ffffffffffffffff RDI: ffffc900005bbd00
2020-05-28T20:47:05.776516-04:00 xtwo70 kernel: RBP: ffffc900005bbd00 R08: 
000000008830cd27 R09: 0000000000000001
2020-05-28T20:47:05.776517-04:00 xtwo70 kernel: R10: 0000000000000001 R11: 
0000000000000003 R12: 0000000000000003
2020-05-28T20:47:05.776518-04:00 xtwo70 kernel: R13: ffff8881327c6818 R14: 
0000000000000040 R15: ffff88814020f020
2020-05-28T20:47:05.776519-04:00 xtwo70 kernel: FS:  00007f1361c6f540(0000) 
GS:ffff888226000000(0000) knlGS:0000000000000000
2020-05-28T20:47:05.776520-04:00 xtwo70 kernel: CS:  0010 DS: 0000 ES: 0000 CR0: 
0000000080050033
2020-05-28T20:47:05.776521-04:00 xtwo70 kernel: CR2: ffff8881327c6868 CR3: 
000000012f48e003 CR4: 00000000003606e0
2020-05-28T20:47:05.776522-04:00 xtwo70 kernel: Call Trace:
2020-05-28T20:47:05.776523-04:00 xtwo70 kernel: xas_load+0xa/0x50
2020-05-28T20:47:05.776524-04:00 xtwo70 kernel: xas_find+0x274/0x2c0
2020-05-28T20:47:05.776525-04:00 xtwo70 kernel: xa_find_after+0x163/0x1f0
2020-05-28T20:47:05.776526-04:00 xtwo70 kernel: scsi_forget_host+0xd2/0x129
2020-05-28T20:47:05.776527-04:00 xtwo70 kernel: scsi_remove_host+0x78/0x120
2020-05-28T20:47:05.776528-04:00 xtwo70 kernel: sdebug_driver_remove+0x3b/0x90 
[scsi_debug]
2020-05-28T20:47:05.776528-04:00 xtwo70 kernel: 
device_release_driver_internal+0xdf/0x1c0
2020-05-28T20:47:05.776529-04:00 xtwo70 kernel: bus_remove_device+0xe4/0x120
2020-05-28T20:47:05.776530-04:00 xtwo70 kernel: device_del+0x174/0x3c0
2020-05-28T20:47:05.776531-04:00 xtwo70 kernel: device_unregister+0x9/0x20
2020-05-28T20:47:05.776532-04:00 xtwo70 kernel: sdebug_do_remove_host+0xc0/0xe0 
[scsi_debug]
2020-05-28T20:47:05.776533-04:00 xtwo70 kernel: scsi_debug_exit+0x21/0xc5b 
[scsi_debug]
2020-05-28T20:47:05.776534-04:00 xtwo70 kernel: __x64_sys_delete_module+0x18f/0x230
2020-05-28T20:47:05.776535-04:00 xtwo70 kernel: ? lockdep_hardirqs_off+0x79/0xd0
2020-05-28T20:47:05.776537-04:00 xtwo70 kernel: ? trace_hardirqs_off_thunk+0x1a/0x1c
2020-05-28T20:47:05.776538-04:00 xtwo70 kernel: do_syscall_64+0x4b/0x1c0
2020-05-28T20:47:05.776539-04:00 xtwo70 kernel: entry_SYSCALL_64_after_hw

[-- Attachment #2: oops.txt --]
[-- Type: text/plain, Size: 10289 bytes --]

2020-05-29T16:26:17.670996-04:00 xtwo70 kernel: scsi 11:0:5:10: Attached scsi generic sg791 type 9
2020-05-29T16:26:17.785102-04:00 xtwo70 kernel: scsi 0:0:0:0: Power-on or device reset occurred
2020-05-29T16:26:17.797144-04:00 xtwo70 kernel: scsi 0:0:0:1: Power-on or device reset occurred
2020-05-29T16:26:18.277606-04:00 xtwo70 kernel: BUG: unable to handle page fault for address: ffff88815c206868
2020-05-29T16:26:18.277633-04:00 xtwo70 kernel: #PF: supervisor read access in kernel mode
2020-05-29T16:26:18.277635-04:00 xtwo70 kernel: #PF: error_code(0x0000) - not-present page
2020-05-29T16:26:18.277640-04:00 xtwo70 kernel: PGD 3c01067 P4D 3c01067 PUD 22ee30067 PMD 22ed4e067 PTE 800ffffea3df9060
2020-05-29T16:26:18.277655-04:00 xtwo70 kernel: Oops: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC PTI
2020-05-29T16:26:18.277671-04:00 xtwo70 kernel: CPU: 0 PID: 6742 Comm: rmmod Not tainted 5.7.0-rc1+ #75
2020-05-29T16:26:18.277672-04:00 xtwo70 kernel: Hardware name: LENOVO 20K5S27Q02/20K5S27Q02, BIOS R0IET61W (1.39 ) 04/20/2020
2020-05-29T16:26:18.277673-04:00 xtwo70 kernel: RIP: 0010:xas_start+0x2a/0x1c0
2020-05-29T16:26:18.277674-04:00 xtwo70 kernel: Code: 41 55 41 54 55 53 48 8b 47 18 48 89 fb 48 89 c2 83 e2 03 0f 84 8d 00 00 00 48 3d 05 c0 ff ff 76 06 48 83 fa 02 74 50 4c 8b 2b <49> 8b 6d 50 e8 fd 2f c2 ff 85 c0 49 89 ec 74 0d 80 3d ef f0 e4 00
2020-05-29T16:26:18.277675-04:00 xtwo70 kernel: RSP: 0018:ffffc900019dfc90 EFLAGS: 00010013
2020-05-29T16:26:18.277676-04:00 xtwo70 kernel: RAX: 0000000000000003 RBX: ffffc900019dfd00 RCX: 000000004a27657b
2020-05-29T16:26:18.277676-04:00 xtwo70 kernel: RDX: 0000000000000003 RSI: ffffffffffffffff RDI: ffffc900019dfd00
2020-05-29T16:26:18.277677-04:00 xtwo70 kernel: RBP: ffffc900019dfd00 R08: 000000001877b6b6 R09: 0000000000000001
2020-05-29T16:26:18.277679-04:00 xtwo70 kernel: R10: 0000000000000001 R11: 0000000000000003 R12: 0000000000000003
2020-05-29T16:26:18.277679-04:00 xtwo70 kernel: R13: ffff88815c206818 R14: 0000000000000040 R15: ffff88815af54020
2020-05-29T16:26:18.277680-04:00 xtwo70 kernel: FS:  00007fbfbb58b540(0000) GS:ffff888225800000(0000) knlGS:0000000000000000
2020-05-29T16:26:18.277681-04:00 xtwo70 kernel: CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
2020-05-29T16:26:18.277681-04:00 xtwo70 kernel: CR2: ffff88815c206868 CR3: 0000000141a5a006 CR4: 00000000003606f0
2020-05-29T16:26:18.277682-04:00 xtwo70 kernel: Call Trace:
2020-05-29T16:26:18.277683-04:00 xtwo70 kernel: xas_load+0xa/0x50
2020-05-29T16:26:18.277683-04:00 xtwo70 kernel: xas_find+0x274/0x2c0
2020-05-29T16:26:18.277684-04:00 xtwo70 kernel: xa_find_after+0x163/0x1f0
2020-05-29T16:26:18.277689-04:00 xtwo70 kernel: scsi_forget_host+0xd2/0x129
2020-05-29T16:26:18.277690-04:00 xtwo70 kernel: scsi_remove_host+0x79/0x120
2020-05-29T16:26:18.277693-04:00 xtwo70 kernel: sdebug_driver_remove+0x3b/0xa0 [scsi_debug]
2020-05-29T16:26:18.277694-04:00 xtwo70 kernel: device_release_driver_internal+0xdf/0x1c0
2020-05-29T16:26:18.277696-04:00 xtwo70 kernel: bus_remove_device+0xe4/0x120
2020-05-29T16:26:18.277697-04:00 xtwo70 kernel: device_del+0x174/0x3c0
2020-05-29T16:26:18.277700-04:00 xtwo70 kernel: device_unregister+0x9/0x20
2020-05-29T16:26:18.277700-04:00 xtwo70 kernel: sdebug_do_remove_host+0xc0/0xe0 [scsi_debug]
2020-05-29T16:26:18.277701-04:00 xtwo70 kernel: scsi_debug_exit+0x21/0xa87 [scsi_debug]
2020-05-29T16:26:18.277706-04:00 xtwo70 kernel: __x64_sys_delete_module+0x18f/0x230
2020-05-29T16:26:18.277712-04:00 xtwo70 kernel: ? lockdep_hardirqs_off+0x79/0xd0
2020-05-29T16:26:18.277715-04:00 xtwo70 kernel: ? trace_hardirqs_off_thunk+0x1a/0x1c
2020-05-29T16:26:18.277721-04:00 xtwo70 kernel: do_syscall_64+0x4b/0x1c0
2020-05-29T16:26:18.277725-04:00 xtwo70 kernel: entry_SYSCALL_64_after_hwframe+0x49/0xb3
2020-05-29T16:26:18.277728-04:00 xtwo70 kernel: RIP: 0033:0x7fbfbb6d7a3b
2020-05-29T16:26:18.277732-04:00 xtwo70 kernel: Code: 73 01 c3 48 8b 0d 55 84 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa b8 b0 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 25 84 0c 00 f7 d8 64 89 01 48
2020-05-29T16:26:18.277735-04:00 xtwo70 kernel: RSP: 002b:00007ffde231d9f8 EFLAGS: 00000206 ORIG_RAX: 00000000000000b0
2020-05-29T16:26:18.277736-04:00 xtwo70 kernel: RAX: ffffffffffffffda RBX: 0000559b8e31f750 RCX: 00007fbfbb6d7a3b
2020-05-29T16:26:18.277736-04:00 xtwo70 kernel: RDX: 000000000000000a RSI: 0000000000000800 RDI: 0000559b8e31f7b8
2020-05-29T16:26:18.277765-04:00 xtwo70 kernel: RBP: 00007ffde231da58 R08: 0000000000000000 R09: 0000000000000000
2020-05-29T16:26:18.277766-04:00 xtwo70 kernel: R10: 00007fbfbb753ac0 R11: 0000000000000206 R12: 00007ffde231dc30
2020-05-29T16:26:18.277767-04:00 xtwo70 kernel: R13: 00007ffde231e8b0 R14: 0000559b8e31f2a0 R15: 0000559b8e31f750
2020-05-29T16:26:18.277767-04:00 xtwo70 kernel: Modules linked in: scsi_debug(-) sg netconsole configfs rfcomm hid_generic hidp hid fuse intel_rapl_msr cmac af_alg bnep iwlmvm intel_rapl_common mac80211 x86_pkg_temp_thermal intel_powerclamp coretemp kvm_intel snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic libarc4 iwlwifi kvm snd_seq_dummy snd_seq_oss i915 snd_seq_midi snd_rawmidi snd_seq_midi_event irqbypass crct10dif_pclmul snd_seq ghash_clmulni_intel snd_hda_intel cfg80211 aesni_intel efi_pstore snd_intel_dspcfg crypto_simd snd_hda_codec snd_hwdep snd_hda_core btusb btrtl cryptd btbcm btintel bluetooth glue_helper nls_iso8859_1 nls_cp437 vfat intel_cstate fat drm_kms_helper intel_uncore ti_usb_3410_5052 mei_me usbserial snd_pcm mmc_block input_leds joydev intel_rapl_perf thinkpad_acpi mousedev mei ecdh_generic serio_raw syscopyarea ecc efivars nvram sysfillrect snd_seq_device ledtrig_audio sysimgblt fb_sys_fops snd_timer intel_gtt intel_xhci_usb_role_switch i2c_algo_bit roles intel_pch_thermal snd soundcore
2020-05-29T16:26:18.277769-04:00 xtwo70 kernel: rfkill evdev video mac_hid nf_log_ipv6 ip6t_REJECT nf_reject_ipv6 xt_hl ip6t_rt nf_log_ipv4 nf_log_common ipt_REJECT nf_reject_ipv4 xt_LOG xt_limit xt_addrtype xt_tcpudp xt_conntrack nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 libcrc32c drm ip6table_filter ip6_tables parport_pc ppdev iptable_filter lp parport drm_panel_orientation_quirks agpgart efivarfs ip_tables x_tables autofs4 rtsx_pci_sdmmc mmc_core xhci_pci xhci_hcd usbcore e1000e nvme i2c_i801 intel_lpss_pci nvme_core intel_lpss rtsx_pci idma64 crc32_pclmul virt_dma usb_common [last unloaded: scsi_debug]
2020-05-29T16:26:18.277770-04:00 xtwo70 kernel: CR2: ffff88815c206868
2020-05-29T16:26:18.277770-04:00 xtwo70 kernel: ---[ end trace e67629a806869a64 ]---
2020-05-29T16:26:18.277771-04:00 xtwo70 kernel: RIP: 0010:xas_start+0x2a/0x1c0
2020-05-29T16:26:18.277771-04:00 xtwo70 kernel: Code: 41 55 41 54 55 53 48 8b 47 18 48 89 fb 48 89 c2 83 e2 03 0f 84 8d 00 00 00 48 3d 05 c0 ff ff 76 06 48 83 fa 02 74 50 4c 8b 2b <49> 8b 6d 50 e8 fd 2f c2 ff 85 c0 49 89 ec 74 0d 80 3d ef f0 e4 00
2020-05-29T16:26:18.277772-04:00 xtwo70 kernel: RSP: 0018:ffffc900019dfc90 EFLAGS: 00010013
2020-05-29T16:26:18.277772-04:00 xtwo70 kernel: RAX: 0000000000000003 RBX: ffffc900019dfd00 RCX: 000000004a27657b
2020-05-29T16:26:18.277773-04:00 xtwo70 kernel: RDX: 0000000000000003 RSI: ffffffffffffffff RDI: ffffc900019dfd00
2020-05-29T16:26:18.277774-04:00 xtwo70 kernel: RBP: ffffc900019dfd00 R08: 000000001877b6b6 R09: 0000000000000001
2020-05-29T16:26:18.277774-04:00 xtwo70 kernel: R10: 0000000000000001 R11: 0000000000000003 R12: 0000000000000003
2020-05-29T16:26:18.277775-04:00 xtwo70 kernel: R13: ffff88815c206818 R14: 0000000000000040 R15: ffff88815af54020
2020-05-29T16:26:18.277775-04:00 xtwo70 kernel: FS:  00007fbfbb58b540(0000) GS:ffff888225800000(0000) knlGS:0000000000000000
2020-05-29T16:26:18.277776-04:00 xtwo70 kernel: CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
2020-05-29T16:26:18.277777-04:00 xtwo70 kernel: CR2: ffff88815c206868 CR3: 0000000141a5a006 CR4: 00000000003606f0
2020-05-29T16:26:18.277777-04:00 xtwo70 kernel: BUG: sleeping function called from invalid context at include/linux/percpu-rwsem.h:49
2020-05-29T16:26:18.277778-04:00 xtwo70 kernel: in_atomic(): 1, irqs_disabled(): 1, non_block: 0, pid: 6742, name: rmmod
2020-05-29T16:26:18.277778-04:00 xtwo70 kernel: INFO: lockdep is turned off.
2020-05-29T16:26:18.277779-04:00 xtwo70 kernel: irq event stamp: 311538
2020-05-29T16:26:18.277780-04:00 xtwo70 kernel: hardirqs last  enabled at (311537): [<ffffffff8125d2d0>] kfree+0x1b0/0x310
2020-05-29T16:26:18.277780-04:00 xtwo70 kernel: hardirqs last disabled at (311538): [<ffffffff818b8be1>] _raw_spin_lock_irqsave+0x11/0x80
2020-05-29T16:26:18.277781-04:00 xtwo70 kernel: softirqs last  enabled at (310482): [<ffffffff81c0039d>] __do_softirq+0x39d/0x4a4
2020-05-29T16:26:18.277782-04:00 xtwo70 kernel: softirqs last disabled at (310473): [<ffffffff8106e7e7>] irq_exit+0xb7/0xd0
2020-05-29T16:26:18.277782-04:00 xtwo70 kernel: Preemption disabled at:
2020-05-29T16:26:18.277783-04:00 xtwo70 kernel: [<0000000000000000>] 0x0
2020-05-29T16:26:18.277784-04:00 xtwo70 kernel: CPU: 0 PID: 6742 Comm: rmmod Tainted: G      D           5.7.0-rc1+ #75
2020-05-29T16:26:18.277784-04:00 xtwo70 kernel: Hardware name: LENOVO 20K5S27Q02/20K5S27Q02, BIOS R0IET61W (1.39 ) 04/20/2020
2020-05-29T16:26:18.277785-04:00 xtwo70 kernel: Call Trace:
2020-05-29T16:26:18.277785-04:00 xtwo70 kernel: dump_stack+0x71/0xa0
2020-05-29T16:26:18.277786-04:00 xtwo70 kernel: ___might_sleep.cold+0xf7/0x10b
2020-05-29T16:26:18.277787-04:00 xtwo70 kernel: exit_signals+0x2b/0x390
2020-05-29T16:26:18.277787-04:00 xtwo70 kernel: do_exit+0xaf/0xb80
2020-05-29T16:26:18.277788-04:00 xtwo70 kernel: ? trace_hardirqs_off_thunk+0x1a/0x1c
2020-05-29T16:26:18.277789-04:00 xtwo70 kernel: rewind_stack_do_exit+0x17/0x20
2020-05-29T16:26:18.277789-04:00 xtwo70 kernel: note: rmmod[6742] exited with preempt_count 1
2020-05-29T16:26:22.525103-04:00 xtwo70 kernel: systemd[1]: systemd-journald-dev-log.socket: Failed to queue service startup job (Maybe the service file is missing or not a non-template unit?): Unit systemd-journald.service is masked.
2020-05-29T16:26:22.526520-04:00 xtwo70 kernel: systemd[1]: systemd-journald-dev-log.socket: Failed with result 'resources'.
2020-05-29T16:26:22.533056-04:00 xtwo70 kernel: systemd[1]: packagekit.service: Succeeded.

Re: [PATCH 2/5] target_core_pscsi: use __scsi_device_lookup()

[Mike Christie]

On 5/29/20 8:47 AM, Hannes Reinecke wrote:
> Instead of walking the list of devices manually use the helper
> function to return the device directly.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> Reviewed-by: Bart van Assche <bvanassche@acm.org>
> ---
>   drivers/target/target_core_pscsi.c | 8 +++-----
>   1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c
> index c9d92b3e777d..5fb852d1281f 100644
> --- a/drivers/target/target_core_pscsi.c
> +++ b/drivers/target/target_core_pscsi.c
> @@ -496,11 +496,9 @@ static int pscsi_configure_device(struct se_device *dev)
>   	}
>   
>   	spin_lock_irq(sh->host_lock);
> -	list_for_each_entry(sd, &sh->__devices, siblings) {
> -		if ((pdv->pdv_channel_id != sd->channel) ||
> -		    (pdv->pdv_target_id != sd->id) ||
> -		    (pdv->pdv_lun_id != sd->lun))
> -			continue;
> +	sd = __scsi_device_lookup(sh, pdv->pdv_channel_id,
> +				  pdv->pdv_target_id, pdv->pdv_lun_id);
> +	if (sd) {
>   		/*
>   		 * Functions will release the held struct scsi_host->host_lock
>   		 * before calling calling pscsi_add_device_to_list() to register
> 

Do you need a check in pscsi_set_configfs_dev_params to make sure 
pdv_channel_id is withing the 16 value limit? If not, userspace could 
use pdv_channel_id = (1 << 17) and when you shift that by 16, will we 
end up with 0 after the cast?

It's probably only going to come up in QA/testing type of setups since 
as you said there's never been a case with a channel_id that large.

Re: [PATCH 5/5] scsi_error: use xarray lookup instead of wrappers

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3094 bytes --]

Hi Hannes,

I love your patch! Perhaps something to improve:

[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on scsi/for-next v5.7-rc7 next-20200529]
[cannot apply to hch-configfs/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Hannes-Reinecke/scsi-use-xarray-for-devices-and-targets/20200531-083913
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 2388a096e7865c043e83ece4e26654bd3d1a20d5)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>, old ones prefixed by <<):

>> drivers/scsi/scsi_error.c:686:15: warning: variable 'starget' is uninitialized when used here [-Wuninitialized]
xa_for_each(&starget->__devices, lun_idx, tmp_sdev) {
^~~~~~~
include/linux/xarray.h:497:20: note: expanded from macro 'xa_for_each'
xa_for_each_start(xa, index, entry, 0)
^~
include/linux/xarray.h:473:20: note: expanded from macro 'xa_for_each_start'
xa_for_each_range(xa, index, entry, start, ULONG_MAX)
^~
include/linux/xarray.h:445:23: note: expanded from macro 'xa_for_each_range'
entry = xa_find(xa, &index, last, XA_PRESENT);                                             ^~
drivers/scsi/scsi_error.c:679:29: note: initialize the variable 'starget' to silence this warning
struct scsi_target *starget;
^
= NULL
1 warning generated.

vim +/starget +686 drivers/scsi/scsi_error.c

   675	
   676	static void scsi_handle_queue_full(struct scsi_device *sdev)
   677	{
   678		struct scsi_host_template *sht = sdev->host->hostt;
   679		struct scsi_target *starget;
   680		struct scsi_device *tmp_sdev;
   681		unsigned long lun_idx = 0;
   682	
   683		if (!sht->track_queue_depth)
   684			return;
   685	
 > 686		xa_for_each(&starget->__devices, lun_idx, tmp_sdev) {
   687			if (tmp_sdev->sdev_state == SDEV_DEL)
   688				continue;
   689			/*
   690			 * We do not know the number of commands that were at
   691			 * the device when we got the queue full so we start
   692			 * from the highest possible value and work our way down.
   693			 */
   694			scsi_track_queue_full(tmp_sdev, tmp_sdev->queue_depth - 1);
   695		}
   696	}
   697	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 73447 bytes --]

Re: [PATCH 5/5] scsi_error: use xarray lookup instead of wrappers

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3170 bytes --]

Hi Hannes,

I love your patch! Perhaps something to improve:

[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on scsi/for-next v5.7-rc7 next-20200529]
[cannot apply to hch-configfs/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url:    https://github.com/0day-ci/linux/commits/Hannes-Reinecke/scsi-use-xarray-for-devices-and-targets/20200531-083913
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 2388a096e7865c043e83ece4e26654bd3d1a20d5)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>, old ones prefixed by <<):

>> drivers/scsi/scsi_error.c:686:15: warning: variable 'starget' is uninitialized when used here [-Wuninitialized]
xa_for_each(&starget->__devices, lun_idx, tmp_sdev) {
^~~~~~~
include/linux/xarray.h:497:20: note: expanded from macro 'xa_for_each'
xa_for_each_start(xa, index, entry, 0)
^~
include/linux/xarray.h:473:20: note: expanded from macro 'xa_for_each_start'
xa_for_each_range(xa, index, entry, start, ULONG_MAX)
^~
include/linux/xarray.h:445:23: note: expanded from macro 'xa_for_each_range'
entry = xa_find(xa, &index, last, XA_PRESENT);                                             ^~
drivers/scsi/scsi_error.c:679:29: note: initialize the variable 'starget' to silence this warning
struct scsi_target *starget;
^
= NULL
1 warning generated.

vim +/starget +686 drivers/scsi/scsi_error.c

   675	
   676	static void scsi_handle_queue_full(struct scsi_device *sdev)
   677	{
   678		struct scsi_host_template *sht = sdev->host->hostt;
   679		struct scsi_target *starget;
   680		struct scsi_device *tmp_sdev;
   681		unsigned long lun_idx = 0;
   682	
   683		if (!sht->track_queue_depth)
   684			return;
   685	
 > 686		xa_for_each(&starget->__devices, lun_idx, tmp_sdev) {
   687			if (tmp_sdev->sdev_state == SDEV_DEL)
   688				continue;
   689			/*
   690			 * We do not know the number of commands that were at
   691			 * the device when we got the queue full so we start
   692			 * from the highest possible value and work our way down.
   693			 */
   694			scsi_track_queue_full(tmp_sdev, tmp_sdev->queue_depth - 1);
   695		}
   696	}
   697	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 73447 bytes --]