* [RFC PATCH 0/4] scsi: use xarray for devices and targets
@ 2020-05-27 14:13 Hannes Reinecke
2020-05-27 14:13 ` [PATCH 1/4] scsi: convert target lookup to xarray Hannes Reinecke
` (4 more replies)
0 siblings, 5 replies; 22+ messages in thread
From: Hannes Reinecke @ 2020-05-27 14:13 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, Doug Gilbert, Daniel Wagner, James Bottomley,
linux-scsi, 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:
- 'channel' and 'id' 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 LUNs are below 256 (to ensure compability with older
systems). So there we can use the LUN number as the index into
the xarray; for larger LUN numbers we'll allocate a separate
index.
With these change we can implement an efficient lookup mechanism,
devolving into direct lookup for most cases.
And iteration should be as efficient as the current, list-based,
approach.
This is compile-tested only, to give you an impression of the
overall idea and to get the discussion rolling.
Hannes Reinecke (4):
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
drivers/scsi/hosts.c | 3 +-
drivers/scsi/scsi.c | 129 ++++++++++++++++++++++++++++---------
drivers/scsi/scsi_lib.c | 8 +--
drivers/scsi/scsi_scan.c | 66 +++++++++----------
drivers/scsi/scsi_sysfs.c | 39 +++++++----
drivers/target/target_core_pscsi.c | 8 +--
include/scsi/scsi_device.h | 21 +++---
include/scsi/scsi_host.h | 5 +-
8 files changed, 175 insertions(+), 104 deletions(-)
--
2.16.4
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 1/4] scsi: convert target lookup to xarray
2020-05-27 14:13 [RFC PATCH 0/4] scsi: use xarray for devices and targets Hannes Reinecke
@ 2020-05-27 14:13 ` Hannes Reinecke
2020-05-27 14:57 ` Johannes Thumshirn
` (3 more replies)
2020-05-27 14:13 ` [PATCH 2/4] target_core_pscsi: use __scsi_device_lookup() Hannes Reinecke
` (3 subsequent siblings)
4 siblings, 4 replies; 22+ messages in thread
From: Hannes Reinecke @ 2020-05-27 14:13 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, Doug Gilbert, Daniel Wagner, James Bottomley,
linux-scsi, 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 | 2 +-
drivers/scsi/scsi.c | 35 ++++++++++++++++++++++++-----------
drivers/scsi/scsi_scan.c | 39 +++++++++++++--------------------------
drivers/scsi/scsi_sysfs.c | 15 +++++++++++----
include/scsi/scsi_device.h | 4 ++--
include/scsi/scsi_host.h | 2 +-
6 files changed, 52 insertions(+), 45 deletions(-)
diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 7ec91c3a66ca..7109afad0183 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -383,7 +383,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(&shost->__targets);
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..25c815355d1a 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.
@@ -670,8 +683,8 @@ EXPORT_SYMBOL(__scsi_device_lookup_by_target);
struct scsi_device *scsi_device_lookup_by_target(struct scsi_target *starget,
u64 lun)
{
- struct scsi_device *sdev;
struct Scsi_Host *shost = dev_to_shost(starget->dev.parent);
+ struct scsi_device *sdev;
unsigned long flags;
spin_lock_irqsave(shost->host_lock, flags);
@@ -701,19 +714,19 @@ 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_target *starget;
struct scsi_device *sdev;
- 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;
- }
+ starget = __scsi_target_lookup(shost, channel, id);
+ if (!starget)
+ return NULL;
+ sdev = __scsi_device_lookup_by_target(starget, lun);
+ if (sdev && sdev->sdev_state == SDEV_DEL)
+ sdev = NULL;
- return NULL;
+ return sdev;
}
EXPORT_SYMBOL(__scsi_device_lookup);
@@ -729,7 +742,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..c47cddef1839 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -304,11 +304,15 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget,
return NULL;
}
+#define scsi_target_index(s) \
+ ((((unsigned long)(s)->channel) << 16) | (s)->id)
+
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 +320,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 +345,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 +400,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 +417,22 @@ 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);
+ found_target = xa_load(&shost->__targets, tid);
if (found_target)
goto found;
-
- list_add_tail(&starget->siblings, &shost->__targets);
+ if (xa_insert(&shost->__targets, tid, starget, GFP_KERNEL)) {
+ dev_printk(KERN_ERR, dev, "target index busy\n");
+ kfree(starget);
+ return NULL;
+ }
spin_unlock_irqrestore(shost->host_lock, flags);
/* allocate and add */
transport_setup_device(dev);
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 163dbcb741c1..95aaa96ce03b 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1512,15 +1512,19 @@ 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) {
+ starget = xa_find(&shost->__targets, &tid, ULONG_MAX, XA_PRESENT);
+ while (starget) {
if (starget->state == STARGET_DEL ||
starget->state == STARGET_REMOVE ||
- starget->state == STARGET_CREATED_REMOVE)
+ starget->state == STARGET_CREATED_REMOVE) {
+ starget = xa_find_after(&shost->__targets, &tid,
+ ULONG_MAX, XA_PRESENT);
continue;
+ }
if (starget->dev.parent == dev || &starget->dev == dev) {
kref_get(&starget->reap_ref);
if (starget->state == STARGET_CREATED)
@@ -1530,7 +1534,10 @@ 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);
+ starget = xa_find_after(&shost->__targets, &tid,
+ ULONG_MAX, XA_PRESENT);
+ continue;
}
}
spin_unlock_irqrestore(shost->host_lock, flags);
diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h
index c3cba2aaf934..28034cc0fce5 100644
--- a/include/scsi/scsi_device.h
+++ b/include/scsi/scsi_device.h
@@ -345,9 +345,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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 2/4] target_core_pscsi: use __scsi_device_lookup()
2020-05-27 14:13 [RFC PATCH 0/4] scsi: use xarray for devices and targets Hannes Reinecke
2020-05-27 14:13 ` [PATCH 1/4] scsi: convert target lookup to xarray Hannes Reinecke
@ 2020-05-27 14:13 ` Hannes Reinecke
2020-05-27 15:05 ` Johannes Thumshirn
2020-05-27 14:13 ` [PATCH 3/4] scsi: move target device list to xarray Hannes Reinecke
` (2 subsequent siblings)
4 siblings, 1 reply; 22+ messages in thread
From: Hannes Reinecke @ 2020-05-27 14:13 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, Doug Gilbert, Daniel Wagner, James Bottomley,
linux-scsi, 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>
---
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 4e37fa9b409d..38799e47b590 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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 3/4] scsi: move target device list to xarray
2020-05-27 14:13 [RFC PATCH 0/4] scsi: use xarray for devices and targets Hannes Reinecke
2020-05-27 14:13 ` [PATCH 1/4] scsi: convert target lookup to xarray Hannes Reinecke
2020-05-27 14:13 ` [PATCH 2/4] target_core_pscsi: use __scsi_device_lookup() Hannes Reinecke
@ 2020-05-27 14:13 ` Hannes Reinecke
2020-05-27 15:34 ` Johannes Thumshirn
` (2 more replies)
2020-05-27 14:14 ` [PATCH 4/4] scsi: remove direct device lookup per host Hannes Reinecke
2020-05-27 16:36 ` [RFC PATCH 0/4] scsi: use xarray for devices and targets Bart Van Assche
4 siblings, 3 replies; 22+ messages in thread
From: Hannes Reinecke @ 2020-05-27 14:13 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, Doug Gilbert, Daniel Wagner, James Bottomley,
linux-scsi, 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>
---
drivers/scsi/scsi.c | 28 +++++++++++++++-------------
drivers/scsi/scsi_lib.c | 8 +++-----
drivers/scsi/scsi_scan.c | 7 +++++--
drivers/scsi/scsi_sysfs.c | 15 +++++++++++++--
include/scsi/scsi_device.h | 4 ++--
5 files changed, 38 insertions(+), 24 deletions(-)
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 25c815355d1a..36aec2b37caa 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_id = 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_id, 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_id = 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_id, sdev)
+ fn(sdev, data);
}
EXPORT_SYMBOL(__starget_for_each_device);
@@ -659,11 +657,15 @@ struct scsi_device *__scsi_device_lookup_by_target(struct scsi_target *starget,
u64 lun)
{
struct scsi_device *sdev;
+ unsigned long lun_idx = 0;
+
+ if (lun < 256)
+ return xa_load(&starget->devices, lun);
- 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 c163fa22267c..23c99fbe47d5 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -361,9 +361,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_id = 0;
spin_lock_irqsave(shost->host_lock, flags);
starget->starget_sdev_user = NULL;
@@ -380,8 +380,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_id, sdev) {
if (sdev == current_sdev)
continue;
if (scsi_device_get(sdev))
@@ -390,7 +389,6 @@ 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 c47cddef1839..6fce5fe6ef32 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -232,10 +232,13 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget,
sdev->id = starget->id;
sdev->lun = lun;
sdev->channel = starget->channel;
+ if (lun < 256)
+ sdev->lun_idx = lun;
+ else
+ sdev->lun_idx = (unsigned int)-1;
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);
@@ -417,7 +420,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(&starget->devices);
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 95aaa96ce03b..b9ed56d6bd95 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 = sdev->sdev_target;
struct device *parent;
struct list_head *this, *tmp;
struct scsi_vpd *vpd_pg80 = NULL, *vpd_pg83 = NULL;
@@ -448,7 +449,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);
@@ -1621,7 +1622,17 @@ 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_idx != (unsigned long)-1)
+ WARN_ON(!xa_insert(&starget->devices, sdev->lun_idx,
+ sdev, GFP_KERNEL));
+ else {
+ struct xa_limit scsi_lun_limit = {
+ .min = 256,
+ .max = UINT_MAX,
+ };
+ WARN_ON(!xa_alloc(&starget->devices, &sdev->lun_idx,
+ sdev, scsi_lun_limit, GFP_KERNEL));
+ }
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 28034cc0fce5..2c6b9d8bc40e 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 {
unsigned int 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 */
unsigned int channel;
--
2.16.4
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 4/4] scsi: remove direct device lookup per host
2020-05-27 14:13 [RFC PATCH 0/4] scsi: use xarray for devices and targets Hannes Reinecke
` (2 preceding siblings ...)
2020-05-27 14:13 ` [PATCH 3/4] scsi: move target device list to xarray Hannes Reinecke
@ 2020-05-27 14:14 ` Hannes Reinecke
2020-05-28 8:00 ` kbuild test robot
2020-05-27 16:36 ` [RFC PATCH 0/4] scsi: use xarray for devices and targets Bart Van Assche
4 siblings, 1 reply; 22+ messages in thread
From: Hannes Reinecke @ 2020-05-27 14:14 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, Doug Gilbert, Daniel Wagner, James Bottomley,
linux-scsi, 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 | 68 +++++++++++++++++++++++++++++++++++++++++-----
drivers/scsi/scsi_scan.c | 20 ++++++++------
drivers/scsi/scsi_sysfs.c | 13 ++-------
include/scsi/scsi_device.h | 13 +++++----
include/scsi/scsi_host.h | 3 +-
6 files changed, 85 insertions(+), 33 deletions(-)
diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 7109afad0183..004a50a95560 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -382,7 +382,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(&shost->__targets);
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 36aec2b37caa..172767e7ee71 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -554,18 +554,39 @@ 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 (!prev) {
+ starget = xa_find(&shost->__targets, &tid,
+ ULONG_MAX, XA_PRESENT);
+ if (starget) {
+ next = xa_find(&starget->devices, &lun_idx,
+ ULONG_MAX, XA_PRESENT);
+ if (!scsi_device_get(next)) {
+ spin_unlock_irqrestore(shost->host_lock, flags);
+ return next;
+ }
+ }
+ } else {
+ starget = prev->sdev_target;
+ lun_idx = prev->lun_idx;
+ }
+ while (starget) {
+ tid = (starget->channel << 16) | starget->id;
+ next = xa_find_after(&starget->devices, &tid,
+ ULONG_MAX, XA_PRESENT);
+ if (!next) {
+ lun_idx = 0;
+ starget = xa_find_after(&shost->__targets, &tid,
+ ULONG_MAX, XA_PRESENT);
+ continue;
+ }
if (!scsi_device_get(next))
break;
- next = NULL;
- list = list->next;
}
spin_unlock_irqrestore(shost->host_lock, flags);
@@ -575,6 +596,38 @@ 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,
+ ULONG_MAX, XA_PRESENT);
+ if (starget)
+ return xa_find(&starget->devices, &lun_idx,
+ ULONG_MAX, XA_PRESENT);
+ } else {
+ starget = prev->sdev_target;
+ lun_idx = prev->lun_idx;
+ }
+ while (starget) {
+ tid = (starget->channel << 16) | starget->id;
+ next = xa_find_after(&starget->devices, &tid,
+ ULONG_MAX, XA_PRESENT);
+ if (next)
+ return next;
+ lun_idx = 0;
+ starget = xa_find_after(&shost->__targets, &tid,
+ ULONG_MAX, XA_PRESENT);
+ }
+ return NULL;
+}
+EXPORT_SYMBOL(__scsi_iterate_devices_unlocked);
+
/**
* __scsi_target_lookup - find a target based on channel and target id
* @shost: SCSI host pointer
@@ -582,11 +635,12 @@ EXPORT_SYMBOL(__scsi_iterate_devices);
* @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);
}
+EXPORT_SYMBOL(__scsi_target_lookup);
/**
* starget_for_each_device - helper to walk all devices of a target
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 6fce5fe6ef32..24179591848b 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -238,7 +238,6 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget,
sdev->lun_idx = (unsigned int)-1;
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);
@@ -1849,17 +1848,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 b9ed56d6bd95..24822f6ee136 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -434,7 +434,6 @@ 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 = sdev->sdev_target;
struct device *parent;
struct list_head *this, *tmp;
struct scsi_vpd *vpd_pg80 = NULL, *vpd_pg83 = NULL;
@@ -448,8 +447,7 @@ 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);
+ xa_erase(&sdev->sdev_target->devices, sdev->lun_idx);
list_del(&sdev->starved_entry);
spin_unlock_irqrestore(sdev->host->host_lock, flags);
@@ -1475,19 +1473,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))
@@ -1496,7 +1491,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);
}
@@ -1633,7 +1627,6 @@ void scsi_sysfs_device_initialize(struct scsi_device *sdev)
WARN_ON(!xa_alloc(&starget->devices, &sdev->lun_idx,
sdev, scsi_lun_limit, GFP_KERNEL));
}
- 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 2c6b9d8bc40e..b811b5e3adfe 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. */
@@ -376,6 +373,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
@@ -389,8 +390,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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] scsi: convert target lookup to xarray
2020-05-27 14:13 ` [PATCH 1/4] scsi: convert target lookup to xarray Hannes Reinecke
@ 2020-05-27 14:57 ` Johannes Thumshirn
2020-05-27 15:06 ` Hannes Reinecke
2020-05-28 7:24 ` Daniel Wagner
` (2 subsequent siblings)
3 siblings, 1 reply; 22+ messages in thread
From: Johannes Thumshirn @ 2020-05-27 14:57 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, Doug Gilbert, Daniel Wagner, James Bottomley,
linux-scsi
On 27/05/2020 16:14, Hannes Reinecke wrote:
[...]
> @@ -670,8 +683,8 @@ EXPORT_SYMBOL(__scsi_device_lookup_by_target);
> struct scsi_device *scsi_device_lookup_by_target(struct scsi_target *starget,
> u64 lun)
> {
> - struct scsi_device *sdev;
> struct Scsi_Host *shost = dev_to_shost(starget->dev.parent);
> + struct scsi_device *sdev;
> unsigned long flags;
This looks unrelated.
>
> spin_lock_irqsave(shost->host_lock, flags);
> @@ -701,19 +714,19 @@ 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_target *starget;
> struct scsi_device *sdev;
>
> - 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;
> - }
> + starget = __scsi_target_lookup(shost, channel, id);
> + if (!starget)
> + return NULL;
> + sdev = __scsi_device_lookup_by_target(starget, lun);
> + if (sdev && sdev->sdev_state == SDEV_DEL)
> + sdev = NULL;
>
I think the above if is unneeded as __scsi_device_lookup_by_target() does:
list_for_each_entry(sdev, &starget->devices, same_target_siblings) {
if (sdev->sdev_state == SDEV_DEL)
continue;
So 'sdev != NULL && sdev->sdev_state == SDEV_DEL' would never evaluate to true,
wouldn't it?
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 2/4] target_core_pscsi: use __scsi_device_lookup()
2020-05-27 14:13 ` [PATCH 2/4] target_core_pscsi: use __scsi_device_lookup() Hannes Reinecke
@ 2020-05-27 15:05 ` Johannes Thumshirn
0 siblings, 0 replies; 22+ messages in thread
From: Johannes Thumshirn @ 2020-05-27 15:05 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, Doug Gilbert, Daniel Wagner, James Bottomley,
linux-scsi
On 27/05/2020 16:14, Hannes Reinecke wrote:
> Instead of walking the list of devices manually use the helper
> function to return the device directly.
I think this a nice cleanup irrespective of the rest of the series.
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] scsi: convert target lookup to xarray
2020-05-27 14:57 ` Johannes Thumshirn
@ 2020-05-27 15:06 ` Hannes Reinecke
0 siblings, 0 replies; 22+ messages in thread
From: Hannes Reinecke @ 2020-05-27 15:06 UTC (permalink / raw)
To: Johannes Thumshirn, Martin K. Petersen
Cc: Christoph Hellwig, Doug Gilbert, Daniel Wagner, James Bottomley,
linux-scsi
On 5/27/20 4:57 PM, Johannes Thumshirn wrote:
> On 27/05/2020 16:14, Hannes Reinecke wrote:
> [...]
>> @@ -670,8 +683,8 @@ EXPORT_SYMBOL(__scsi_device_lookup_by_target);
>> struct scsi_device *scsi_device_lookup_by_target(struct scsi_target *starget,
>> u64 lun)
>> {
>> - struct scsi_device *sdev;
>> struct Scsi_Host *shost = dev_to_shost(starget->dev.parent);
>> + struct scsi_device *sdev;
>> unsigned long flags;
>
> This looks unrelated.
>
>>
>> spin_lock_irqsave(shost->host_lock, flags);
>> @@ -701,19 +714,19 @@ 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_target *starget;
>> struct scsi_device *sdev;
>>
>> - 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;
>> - }
>> + starget = __scsi_target_lookup(shost, channel, id);
>> + if (!starget)
>> + return NULL;
>> + sdev = __scsi_device_lookup_by_target(starget, lun);
>> + if (sdev && sdev->sdev_state == SDEV_DEL)
>> + sdev = NULL;
>>
>
> I think the above if is unneeded as __scsi_device_lookup_by_target() does:
> list_for_each_entry(sdev, &starget->devices, same_target_siblings) {
> if (sdev->sdev_state == SDEV_DEL)
> continue;
>
> So 'sdev != NULL && sdev->sdev_state == SDEV_DEL' would never evaluate to true,
> wouldn't it?
>
Oh. indeed. So that can be simplified.
Will do for the next version.
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] scsi: move target device list to xarray
2020-05-27 14:13 ` [PATCH 3/4] scsi: move target device list to xarray Hannes Reinecke
@ 2020-05-27 15:34 ` Johannes Thumshirn
2020-05-27 20:13 ` kbuild test robot
2020-05-30 2:47 ` kbuild test robot
2 siblings, 0 replies; 22+ messages in thread
From: Johannes Thumshirn @ 2020-05-27 15:34 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, Doug Gilbert, Daniel Wagner, James Bottomley,
linux-scsi
Looks good (I think),
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH 0/4] scsi: use xarray for devices and targets
2020-05-27 14:13 [RFC PATCH 0/4] scsi: use xarray for devices and targets Hannes Reinecke
` (3 preceding siblings ...)
2020-05-27 14:14 ` [PATCH 4/4] scsi: remove direct device lookup per host Hannes Reinecke
@ 2020-05-27 16:36 ` Bart Van Assche
2020-05-27 16:59 ` Hannes Reinecke
2020-05-28 3:59 ` Douglas Gilbert
4 siblings, 2 replies; 22+ messages in thread
From: Bart Van Assche @ 2020-05-27 16:36 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, Doug Gilbert, Daniel Wagner, James Bottomley,
linux-scsi
On 2020-05-27 07:13, 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:
> - 'channel' and 'id' 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 LUNs are below 256 (to ensure compability with older
> systems). So there we can use the LUN number as the index into
> the xarray; for larger LUN numbers we'll allocate a separate
> index.
>
> With these change we can implement an efficient lookup mechanism,
> devolving into direct lookup for most cases.
> And iteration should be as efficient as the current, list-based,
> approach.
>
> This is compile-tested only, to give you an impression of the
> overall idea and to get the discussion rolling.
Hi Hannes,
My understanding of the xarray concept is that it provides two
advantages over using linked lists:
- Faster lookups.
- Requires less memory.
Will we benefit from any of these advantages in the SCSI code? Hadn't
James Bottomley already brought up that lookup by (channel, target, lun)
only happens from some LLDs and from the procfs code?
Are there any use cases where the number of SCSI devices is large enough
to benefit from the memory reduction?
Thanks,
Bart.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH 0/4] scsi: use xarray for devices and targets
2020-05-27 16:36 ` [RFC PATCH 0/4] scsi: use xarray for devices and targets Bart Van Assche
@ 2020-05-27 16:59 ` Hannes Reinecke
2020-05-28 3:59 ` Douglas Gilbert
1 sibling, 0 replies; 22+ messages in thread
From: Hannes Reinecke @ 2020-05-27 16:59 UTC (permalink / raw)
To: Bart Van Assche, Martin K. Petersen
Cc: Christoph Hellwig, Doug Gilbert, Daniel Wagner, James Bottomley,
linux-scsi
On 5/27/20 6:36 PM, Bart Van Assche wrote:
> On 2020-05-27 07:13, 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:
>> - 'channel' and 'id' 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 LUNs are below 256 (to ensure compability with older
>> systems). So there we can use the LUN number as the index into
>> the xarray; for larger LUN numbers we'll allocate a separate
>> index.
>>
>> With these change we can implement an efficient lookup mechanism,
>> devolving into direct lookup for most cases.
>> And iteration should be as efficient as the current, list-based,
>> approach.
>>
>> This is compile-tested only, to give you an impression of the
>> overall idea and to get the discussion rolling.
>
> Hi Hannes,
>
> My understanding of the xarray concept is that it provides two
> advantages over using linked lists:
> - Faster lookups.
> - Requires less memory.
>
> Will we benefit from any of these advantages in the SCSI code? Hadn't
> James Bottomley already brought up that lookup by (channel, target, lun)
> only happens from some LLDs and from the procfs code?
>
It's not only lookup, it's iteration in general.
Which affects scanning and device removal; especially the latter is
_very_ error prone (just look at scsi_target_reap etc), so any reduction
in complexity is a good thing in general methinks.
> Are there any use cases where the number of SCSI devices is large enough
> to benefit from the memory reduction?
>
I would assume that we're seeing benefits as soon as we're in the range
of tens to hundreds of devices; then list lookup will be eating up more
time and space as xarrays.
And the big benefit of using xarrays is that we will be alerted if an
element with the same indices is being added; we've already had issues
in the past here which are notoriously difficult to track down.
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] scsi: move target device list to xarray
2020-05-27 14:13 ` [PATCH 3/4] scsi: move target device list to xarray Hannes Reinecke
2020-05-27 15:34 ` Johannes Thumshirn
@ 2020-05-27 20:13 ` kbuild test robot
2020-05-30 2:47 ` kbuild test robot
2 siblings, 0 replies; 22+ messages in thread
From: kbuild test robot @ 2020-05-27 20:13 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: kbuild-all, Christoph Hellwig, Doug Gilbert, Daniel Wagner,
James Bottomley, linux-scsi, Hannes Reinecke
[-- Attachment #1: Type: text/plain, Size: 4193 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-20200526]
[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/20200527-231824
base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 9.3.0
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
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=ia64
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_sysfs.c: In function 'scsi_sysfs_device_initialize':
>> drivers/scsi/scsi_sysfs.c:1625:20: warning: comparison is always true due to limited range of data type [-Wtype-limits]
1625 | if (sdev->lun_idx != (unsigned long)-1)
| ^~
drivers/scsi/scsi_sysfs.c: In function 'scsi_device_dev_release_usercontext':
drivers/scsi/scsi_sysfs.c:437:22: warning: 'sdev' is used uninitialized in this function [-Wuninitialized]
437 | struct scsi_target *starget = sdev->sdev_target;
| ^~~~~~~
vim +1625 drivers/scsi/scsi_sysfs.c
1592
1593 void scsi_sysfs_device_initialize(struct scsi_device *sdev)
1594 {
1595 unsigned long flags;
1596 struct Scsi_Host *shost = sdev->host;
1597 struct scsi_target *starget = sdev->sdev_target;
1598
1599 device_initialize(&sdev->sdev_gendev);
1600 sdev->sdev_gendev.bus = &scsi_bus_type;
1601 sdev->sdev_gendev.type = &scsi_dev_type;
1602 dev_set_name(&sdev->sdev_gendev, "%d:%d:%d:%llu",
1603 sdev->host->host_no, sdev->channel, sdev->id, sdev->lun);
1604
1605 device_initialize(&sdev->sdev_dev);
1606 sdev->sdev_dev.parent = get_device(&sdev->sdev_gendev);
1607 sdev->sdev_dev.class = &sdev_class;
1608 dev_set_name(&sdev->sdev_dev, "%d:%d:%d:%llu",
1609 sdev->host->host_no, sdev->channel, sdev->id, sdev->lun);
1610 /*
1611 * Get a default scsi_level from the target (derived from sibling
1612 * devices). This is the best we can do for guessing how to set
1613 * sdev->lun_in_cdb for the initial INQUIRY command. For LUN 0 the
1614 * setting doesn't matter, because all the bits are zero anyway.
1615 * But it does matter for higher LUNs.
1616 */
1617 sdev->scsi_level = starget->scsi_level;
1618 if (sdev->scsi_level <= SCSI_2 &&
1619 sdev->scsi_level != SCSI_UNKNOWN &&
1620 !shost->no_scsi2_lun_in_cdb)
1621 sdev->lun_in_cdb = 1;
1622
1623 transport_setup_device(&sdev->sdev_gendev);
1624 spin_lock_irqsave(shost->host_lock, flags);
> 1625 if (sdev->lun_idx != (unsigned long)-1)
1626 WARN_ON(!xa_insert(&starget->devices, sdev->lun_idx,
1627 sdev, GFP_KERNEL));
1628 else {
1629 struct xa_limit scsi_lun_limit = {
1630 .min = 256,
1631 .max = UINT_MAX,
1632 };
1633 WARN_ON(!xa_alloc(&starget->devices, &sdev->lun_idx,
1634 sdev, scsi_lun_limit, GFP_KERNEL));
1635 }
1636 list_add_tail(&sdev->siblings, &shost->__devices);
1637 spin_unlock_irqrestore(shost->host_lock, flags);
1638 /*
1639 * device can now only be removed via __scsi_remove_device() so hold
1640 * the target. Target will be held in CREATED state until something
1641 * beneath it becomes visible (in which case it moves to RUNNING)
1642 */
1643 kref_get(&starget->reap_ref);
1644 }
1645
---
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: 57752 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC PATCH 0/4] scsi: use xarray for devices and targets
2020-05-27 16:36 ` [RFC PATCH 0/4] scsi: use xarray for devices and targets Bart Van Assche
2020-05-27 16:59 ` Hannes Reinecke
@ 2020-05-28 3:59 ` Douglas Gilbert
1 sibling, 0 replies; 22+ messages in thread
From: Douglas Gilbert @ 2020-05-28 3:59 UTC (permalink / raw)
To: Bart Van Assche, Hannes Reinecke, Martin K. Petersen
Cc: Christoph Hellwig, Daniel Wagner, James Bottomley, linux-scsi,
Matthew Wilcox, Ming Lei
On 2020-05-27 12:36 p.m., Bart Van Assche wrote:
> On 2020-05-27 07:13, 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:
>> - 'channel' and 'id' 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 LUNs are below 256 (to ensure compability with older
>> systems). So there we can use the LUN number as the index into
>> the xarray; for larger LUN numbers we'll allocate a separate
>> index.
>>
>> With these change we can implement an efficient lookup mechanism,
>> devolving into direct lookup for most cases.
>> And iteration should be as efficient as the current, list-based,
>> approach.
>>
>> This is compile-tested only, to give you an impression of the
>> overall idea and to get the discussion rolling.
>
> Hi Hannes,
>
> My understanding of the xarray concept is that it provides two
> advantages over using linked lists:
> - Faster lookups.
> - Requires less memory.
Bart,
You might add these:
- sane deletion semantics
- inbuilt locking
- inherently safer iterations, especially if marks are used
Matthew can probably add more.
> Will we benefit from any of these advantages in the SCSI code? Hadn't
> James Bottomley already brought up that lookup by (channel, target, lun)
> only happens from some LLDs and from the procfs code?
The way that the SCSI object tree hangs together with doubly linked
lists may have been a coherent design 20 years ago when JB wrote it,
but it has been white-anted big time since then.
The current state of that code is hard to defend. I have between 10 and 20
more examples of patently stupid things the current code does. See my
exchange with JB concerning the starget iterator over its sdevs that decided
to check on _every_ sdev in that host. That is done in several places.
The redundant sdevs in a shost collection is probably the most glaring
current design flaw.
> Are there any use cases where the number of SCSI devices is large enough
> to benefit from the memory reduction?
I don't believe that overall memory usage is a problem. Fitting the sdev_s
of hot devices in a smaller number of cache lines would help. That is
where Ming Lei was looking that kicked off this exercise that has
morphed into using xarray.
Doug Gilbert
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] scsi: convert target lookup to xarray
2020-05-27 14:13 ` [PATCH 1/4] scsi: convert target lookup to xarray Hannes Reinecke
2020-05-27 14:57 ` Johannes Thumshirn
@ 2020-05-28 7:24 ` Daniel Wagner
2020-05-28 7:52 ` Hannes Reinecke
2020-05-29 5:01 ` kbuild test robot
2020-05-31 9:10 ` Dan Carpenter
3 siblings, 1 reply; 22+ messages in thread
From: Daniel Wagner @ 2020-05-28 7:24 UTC (permalink / raw)
To: Hannes Reinecke
Cc: Martin K. Petersen, Christoph Hellwig, Doug Gilbert,
Daniel Wagner, James Bottomley, linux-scsi
On Wed, May 27, 2020 at 04:13:57PM +0200, Hannes Reinecke wrote:
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -1512,15 +1512,19 @@ 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) {
> + starget = xa_find(&shost->__targets, &tid, ULONG_MAX, XA_PRESENT);
> + while (starget) {
> if (starget->state == STARGET_DEL ||
> starget->state == STARGET_REMOVE ||
> - starget->state == STARGET_CREATED_REMOVE)
> + starget->state == STARGET_CREATED_REMOVE) {
> + starget = xa_find_after(&shost->__targets, &tid,
> + ULONG_MAX, XA_PRESENT);
> continue;
> + }
> if (starget->dev.parent == dev || &starget->dev == dev) {
> kref_get(&starget->reap_ref);
> if (starget->state == STARGET_CREATED)
> @@ -1530,7 +1534,10 @@ 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);
> + starget = xa_find_after(&shost->__targets, &tid,
> + ULONG_MAX, XA_PRESENT);
> + continue;
> }
Is there a special reason why don't use xa_for_each to iterate through all
entries?
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] scsi: convert target lookup to xarray
2020-05-28 7:24 ` Daniel Wagner
@ 2020-05-28 7:52 ` Hannes Reinecke
2020-05-28 16:28 ` Bart Van Assche
0 siblings, 1 reply; 22+ messages in thread
From: Hannes Reinecke @ 2020-05-28 7:52 UTC (permalink / raw)
To: Daniel Wagner
Cc: Martin K. Petersen, Christoph Hellwig, Doug Gilbert,
Daniel Wagner, James Bottomley, linux-scsi
On 5/28/20 9:24 AM, Daniel Wagner wrote:
> On Wed, May 27, 2020 at 04:13:57PM +0200, Hannes Reinecke wrote:
>> --- a/drivers/scsi/scsi_sysfs.c
>> +++ b/drivers/scsi/scsi_sysfs.c
>> @@ -1512,15 +1512,19 @@ 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) {
>> + starget = xa_find(&shost->__targets, &tid, ULONG_MAX, XA_PRESENT);
>> + while (starget) {
>> if (starget->state == STARGET_DEL ||
>> starget->state == STARGET_REMOVE ||
>> - starget->state == STARGET_CREATED_REMOVE)
>> + starget->state == STARGET_CREATED_REMOVE) {
>> + starget = xa_find_after(&shost->__targets, &tid,
>> + ULONG_MAX, XA_PRESENT);
>> continue;
>> + }
>> if (starget->dev.parent == dev || &starget->dev == dev) {
>> kref_get(&starget->reap_ref);
>> if (starget->state == STARGET_CREATED)
>> @@ -1530,7 +1534,10 @@ 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);
>> + starget = xa_find_after(&shost->__targets, &tid,
>> + ULONG_MAX, XA_PRESENT);
>> + continue;
>> }
>
> Is there a special reason why don't use xa_for_each to iterate through all
> entries?
>
This entire function is completely daft.
It says 'scsi_remove_target()', but for some weird reason fails to pass
in the target it want to delete, so it has to go round in circles trying
to figure out which target to delete.
There probably had been an obscure reason for this, but with the current
code it's just pointless.
So that's the next thing to fix (after all of this): use a struct
scsi_target as argument for this function, then this entire loop can go.
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 4/4] scsi: remove direct device lookup per host
2020-05-27 14:14 ` [PATCH 4/4] scsi: remove direct device lookup per host Hannes Reinecke
@ 2020-05-28 8:00 ` kbuild test robot
0 siblings, 0 replies; 22+ messages in thread
From: kbuild test robot @ 2020-05-28 8:00 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: kbuild-all, Christoph Hellwig, Doug Gilbert, Daniel Wagner,
James Bottomley, linux-scsi, Hannes Reinecke
[-- Attachment #1: Type: text/plain, Size: 1836 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-20200526]
[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/20200527-231824
base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: x86_64-lkp (attached as .config)
compiler: gcc-8 (Debian 8.3.0-6) 8.3.0
reproduce (this is a W=1 build):
# save the attached .config to linux build tree
make W=1 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.c:638:21: warning: no previous prototype for '__scsi_target_lookup' [-Wmissing-prototypes]
struct scsi_target *__scsi_target_lookup(struct Scsi_Host *shost,
^~~~~~~~~~~~~~~~~~~~
vim +/__scsi_target_lookup +638 drivers/scsi/scsi.c
630
631 /**
632 * __scsi_target_lookup - find a target based on channel and target id
633 * @shost: SCSI host pointer
634 * @channel: channel number of the target
635 * @id: ID of the target
636 *
637 */
> 638 struct scsi_target *__scsi_target_lookup(struct Scsi_Host *shost,
639 u16 channel, u16 id)
640 {
641 return xa_load(&shost->__targets, (channel << 16) | id);
642 }
643 EXPORT_SYMBOL(__scsi_target_lookup);
644
---
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: 28414 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] scsi: convert target lookup to xarray
2020-05-28 7:52 ` Hannes Reinecke
@ 2020-05-28 16:28 ` Bart Van Assche
0 siblings, 0 replies; 22+ messages in thread
From: Bart Van Assche @ 2020-05-28 16:28 UTC (permalink / raw)
To: Hannes Reinecke, Daniel Wagner
Cc: Martin K. Petersen, Christoph Hellwig, Doug Gilbert,
Daniel Wagner, James Bottomley, linux-scsi
On 2020-05-28 00:52, Hannes Reinecke wrote:
> On 5/28/20 9:24 AM, Daniel Wagner wrote:
>> Is there a special reason why don't use xa_for_each to iterate through
>> all
>> entries?
>>
> This entire function is completely daft.
> It says 'scsi_remove_target()', but for some weird reason fails to pass
> in the target it want to delete, so it has to go round in circles trying
> to figure out which target to delete.
> There probably had been an obscure reason for this, but with the current
> code it's just pointless.
> So that's the next thing to fix (after all of this): use a struct
> scsi_target as argument for this function, then this entire loop can go.
Please be careful when modifying this code. I remember that it took
multiple iterations to get this code right. See e.g. commit 81b6c9998979
("scsi: core: check for device state in __scsi_remove_target()"). See
also commit fbce4d97fd43 ("scsi: fixup kernel warning during rmmod()").
See also commit f9279c968c25 ("scsi: Add STARGET_CREATED_REMOVE state to
scsi_target_state").
The only reason I can think of why that loop exists is that @dev may be
associated with multiple targets (struct scsi_target) at the same time.
Is that something that can happen?
Thanks,
Bart.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] scsi: convert target lookup to xarray
2020-05-27 14:13 ` [PATCH 1/4] scsi: convert target lookup to xarray Hannes Reinecke
2020-05-27 14:57 ` Johannes Thumshirn
2020-05-28 7:24 ` Daniel Wagner
@ 2020-05-29 5:01 ` kbuild test robot
2020-05-31 9:10 ` Dan Carpenter
3 siblings, 0 replies; 22+ messages in thread
From: kbuild test robot @ 2020-05-29 5:01 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: kbuild-all, Christoph Hellwig, Doug Gilbert, Daniel Wagner,
James Bottomley, linux-scsi, Hannes Reinecke
[-- Attachment #1: Type: text/plain, Size: 11065 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-20200526]
[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/20200527-231824
base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
:::::: branch date: 12 hours ago
:::::: commit date: 12 hours ago
config: i386-randconfig-s001-20200528 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-240-gf0fe1cd9-dirty
# save the attached .config to linux build tree
make W=1 C=1 ARCH=i386 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> drivers/scsi/scsi_scan.c:392:27: sparse: sparse: context imbalance in 'scsi_alloc_target' - different lock contexts for basic block
# https://github.com/0day-ci/linux/commit/45b149b239ea9a86968ddbd8ecda1e6c44937b68
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 45b149b239ea9a86968ddbd8ecda1e6c44937b68
vim +/scsi_alloc_target +392 drivers/scsi/scsi_scan.c
e63ed0d7a98014 James Bottomley 2014-01-21 379
884d25cc4fda20 James Bottomley 2006-09-05 380 /**
884d25cc4fda20 James Bottomley 2006-09-05 381 * scsi_alloc_target - allocate a new or find an existing target
884d25cc4fda20 James Bottomley 2006-09-05 382 * @parent: parent of the target (need not be a scsi host)
884d25cc4fda20 James Bottomley 2006-09-05 383 * @channel: target channel number (zero if no channels)
884d25cc4fda20 James Bottomley 2006-09-05 384 * @id: target id number
884d25cc4fda20 James Bottomley 2006-09-05 385 *
884d25cc4fda20 James Bottomley 2006-09-05 386 * Return an existing target if one exists, provided it hasn't already
884d25cc4fda20 James Bottomley 2006-09-05 387 * gone into STARGET_DEL state, otherwise allocate a new target.
884d25cc4fda20 James Bottomley 2006-09-05 388 *
884d25cc4fda20 James Bottomley 2006-09-05 389 * The target is returned with an incremented reference, so the caller
884d25cc4fda20 James Bottomley 2006-09-05 390 * is responsible for both reaping and doing a last put
884d25cc4fda20 James Bottomley 2006-09-05 391 */
^1da177e4c3f41 Linus Torvalds 2005-04-16 @392 static struct scsi_target *scsi_alloc_target(struct device *parent,
^1da177e4c3f41 Linus Torvalds 2005-04-16 393 int channel, uint id)
^1da177e4c3f41 Linus Torvalds 2005-04-16 394 {
^1da177e4c3f41 Linus Torvalds 2005-04-16 395 struct Scsi_Host *shost = dev_to_shost(parent);
^1da177e4c3f41 Linus Torvalds 2005-04-16 396 struct device *dev = NULL;
^1da177e4c3f41 Linus Torvalds 2005-04-16 397 unsigned long flags;
^1da177e4c3f41 Linus Torvalds 2005-04-16 398 const int size = sizeof(struct scsi_target)
^1da177e4c3f41 Linus Torvalds 2005-04-16 399 + shost->transportt->target_size;
5c44cd2afad3f7 James.Smart@Emulex.Com 2005-06-10 400 struct scsi_target *starget;
^1da177e4c3f41 Linus Torvalds 2005-04-16 401 struct scsi_target *found_target;
e63ed0d7a98014 James Bottomley 2014-01-21 402 int error, ref_got;
45b149b239ea9a Hannes Reinecke 2020-05-27 403 unsigned long tid;
^1da177e4c3f41 Linus Torvalds 2005-04-16 404
24669f75a3231f Jes Sorensen 2006-01-16 405 starget = kzalloc(size, GFP_KERNEL);
^1da177e4c3f41 Linus Torvalds 2005-04-16 406 if (!starget) {
cadbd4a5e36dde Harvey Harrison 2008-07-03 407 printk(KERN_ERR "%s: allocation failure\n", __func__);
^1da177e4c3f41 Linus Torvalds 2005-04-16 408 return NULL;
^1da177e4c3f41 Linus Torvalds 2005-04-16 409 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 410 dev = &starget->dev;
^1da177e4c3f41 Linus Torvalds 2005-04-16 411 device_initialize(dev);
e63ed0d7a98014 James Bottomley 2014-01-21 412 kref_init(&starget->reap_ref);
^1da177e4c3f41 Linus Torvalds 2005-04-16 413 dev->parent = get_device(parent);
71610f55fa4db6 Kay Sievers 2008-12-03 414 dev_set_name(dev, "target%d:%d:%d", shost->host_no, channel, id);
b0ed43360fdca2 Hannes Reinecke 2008-03-18 415 dev->bus = &scsi_bus_type;
b0ed43360fdca2 Hannes Reinecke 2008-03-18 416 dev->type = &scsi_target_type;
^1da177e4c3f41 Linus Torvalds 2005-04-16 417 starget->id = id;
^1da177e4c3f41 Linus Torvalds 2005-04-16 418 starget->channel = channel;
f0c0a376d0fcd4 Mike Christie 2008-08-17 419 starget->can_queue = 0;
^1da177e4c3f41 Linus Torvalds 2005-04-16 420 INIT_LIST_HEAD(&starget->devices);
643eb2d932c97a James Bottomley 2008-03-22 421 starget->state = STARGET_CREATED;
7c9d6f16f50d3a Alan Stern 2007-01-08 422 starget->scsi_level = SCSI_2;
c53a284f8be237 Edward Goggin 2009-04-09 423 starget->max_target_blocked = SCSI_DEFAULT_TARGET_BLOCKED;
45b149b239ea9a Hannes Reinecke 2020-05-27 424 tid = scsi_target_index(starget);
ffedb4522571ac James Bottomley 2006-02-23 425 retry:
^1da177e4c3f41 Linus Torvalds 2005-04-16 426 spin_lock_irqsave(shost->host_lock, flags);
^1da177e4c3f41 Linus Torvalds 2005-04-16 427
45b149b239ea9a Hannes Reinecke 2020-05-27 428 found_target = xa_load(&shost->__targets, tid);
^1da177e4c3f41 Linus Torvalds 2005-04-16 429 if (found_target)
^1da177e4c3f41 Linus Torvalds 2005-04-16 430 goto found;
45b149b239ea9a Hannes Reinecke 2020-05-27 431 if (xa_insert(&shost->__targets, tid, starget, GFP_KERNEL)) {
45b149b239ea9a Hannes Reinecke 2020-05-27 432 dev_printk(KERN_ERR, dev, "target index busy\n");
45b149b239ea9a Hannes Reinecke 2020-05-27 433 kfree(starget);
45b149b239ea9a Hannes Reinecke 2020-05-27 434 return NULL;
45b149b239ea9a Hannes Reinecke 2020-05-27 435 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 436 spin_unlock_irqrestore(shost->host_lock, flags);
^1da177e4c3f41 Linus Torvalds 2005-04-16 437 /* allocate and add */
a283bd37d00e92 James Bottomley 2005-05-24 438 transport_setup_device(dev);
a283bd37d00e92 James Bottomley 2005-05-24 439 if (shost->hostt->target_alloc) {
32f95792500794 Brian King 2006-02-22 440 error = shost->hostt->target_alloc(starget);
a283bd37d00e92 James Bottomley 2005-05-24 441
a283bd37d00e92 James Bottomley 2005-05-24 442 if(error) {
a283bd37d00e92 James Bottomley 2005-05-24 443 dev_printk(KERN_ERR, dev, "target allocation failed, error %d\n", error);
a283bd37d00e92 James Bottomley 2005-05-24 444 /* don't want scsi_target_reap to do the final
a283bd37d00e92 James Bottomley 2005-05-24 445 * put because it will be under the host lock */
643eb2d932c97a James Bottomley 2008-03-22 446 scsi_target_destroy(starget);
a283bd37d00e92 James Bottomley 2005-05-24 447 return NULL;
a283bd37d00e92 James Bottomley 2005-05-24 448 }
a283bd37d00e92 James Bottomley 2005-05-24 449 }
884d25cc4fda20 James Bottomley 2006-09-05 450 get_device(dev);
a283bd37d00e92 James Bottomley 2005-05-24 451
^1da177e4c3f41 Linus Torvalds 2005-04-16 452 return starget;
^1da177e4c3f41 Linus Torvalds 2005-04-16 453
^1da177e4c3f41 Linus Torvalds 2005-04-16 454 found:
e63ed0d7a98014 James Bottomley 2014-01-21 455 /*
e63ed0d7a98014 James Bottomley 2014-01-21 456 * release routine already fired if kref is zero, so if we can still
e63ed0d7a98014 James Bottomley 2014-01-21 457 * take the reference, the target must be alive. If we can't, it must
e63ed0d7a98014 James Bottomley 2014-01-21 458 * be dying and we need to wait for a new target
e63ed0d7a98014 James Bottomley 2014-01-21 459 */
e63ed0d7a98014 James Bottomley 2014-01-21 460 ref_got = kref_get_unless_zero(&found_target->reap_ref);
e63ed0d7a98014 James Bottomley 2014-01-21 461
^1da177e4c3f41 Linus Torvalds 2005-04-16 462 spin_unlock_irqrestore(shost->host_lock, flags);
e63ed0d7a98014 James Bottomley 2014-01-21 463 if (ref_got) {
12fb8c1574d7d0 Alan Stern 2010-03-18 464 put_device(dev);
^1da177e4c3f41 Linus Torvalds 2005-04-16 465 return found_target;
^1da177e4c3f41 Linus Torvalds 2005-04-16 466 }
e63ed0d7a98014 James Bottomley 2014-01-21 467 /*
e63ed0d7a98014 James Bottomley 2014-01-21 468 * Unfortunately, we found a dying target; need to wait until it's
e63ed0d7a98014 James Bottomley 2014-01-21 469 * dead before we can get a new one. There is an anomaly here. We
e63ed0d7a98014 James Bottomley 2014-01-21 470 * *should* call scsi_target_reap() to balance the kref_get() of the
e63ed0d7a98014 James Bottomley 2014-01-21 471 * reap_ref above. However, since the target being released, it's
e63ed0d7a98014 James Bottomley 2014-01-21 472 * already invisible and the reap_ref is irrelevant. If we call
e63ed0d7a98014 James Bottomley 2014-01-21 473 * scsi_target_reap() we might spuriously do another device_del() on
e63ed0d7a98014 James Bottomley 2014-01-21 474 * an already invisible target.
e63ed0d7a98014 James Bottomley 2014-01-21 475 */
ffedb4522571ac James Bottomley 2006-02-23 476 put_device(&found_target->dev);
e63ed0d7a98014 James Bottomley 2014-01-21 477 /*
e63ed0d7a98014 James Bottomley 2014-01-21 478 * length of time is irrelevant here, we just want to yield the CPU
e63ed0d7a98014 James Bottomley 2014-01-21 479 * for a tick to avoid busy waiting for the target to die.
e63ed0d7a98014 James Bottomley 2014-01-21 480 */
e63ed0d7a98014 James Bottomley 2014-01-21 481 msleep(1);
ffedb4522571ac James Bottomley 2006-02-23 482 goto retry;
ffedb4522571ac James Bottomley 2006-02-23 483 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 484
:::::: The code at line 392 was first introduced by commit
:::::: 1da177e4c3f41524e886b7f1b8a0c1fc7321cac2 Linux-2.6.12-rc2
:::::: TO: Linus Torvalds <torvalds@ppc970.osdl.org>
:::::: CC: Linus Torvalds <torvalds@ppc970.osdl.org>
---
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: 35320 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 3/4] scsi: move target device list to xarray
2020-05-27 14:13 ` [PATCH 3/4] scsi: move target device list to xarray Hannes Reinecke
2020-05-27 15:34 ` Johannes Thumshirn
2020-05-27 20:13 ` kbuild test robot
@ 2020-05-30 2:47 ` kbuild test robot
2 siblings, 0 replies; 22+ messages in thread
From: kbuild test robot @ 2020-05-30 2:47 UTC (permalink / raw)
To: Hannes Reinecke, Martin K. Petersen
Cc: kbuild-all, Christoph Hellwig, Doug Gilbert, Daniel Wagner,
James Bottomley, linux-scsi, Hannes Reinecke
[-- Attachment #1: Type: text/plain, Size: 3559 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]
[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/20200527-231824
base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: x86_64-randconfig-m001-20200529 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
New smatch warnings:
drivers/scsi/scsi_sysfs.c:1625 scsi_sysfs_device_initialize() warn: always true condition '(sdev->lun_idx != -1) => (0-u32max != u64max)'
Old smatch warnings:
drivers/scsi/scsi_sysfs.c:437 scsi_device_dev_release_usercontext() error: potentially dereferencing uninitialized 'sdev'.
vim +1625 drivers/scsi/scsi_sysfs.c
1592
1593 void scsi_sysfs_device_initialize(struct scsi_device *sdev)
1594 {
1595 unsigned long flags;
1596 struct Scsi_Host *shost = sdev->host;
1597 struct scsi_target *starget = sdev->sdev_target;
1598
1599 device_initialize(&sdev->sdev_gendev);
1600 sdev->sdev_gendev.bus = &scsi_bus_type;
1601 sdev->sdev_gendev.type = &scsi_dev_type;
1602 dev_set_name(&sdev->sdev_gendev, "%d:%d:%d:%llu",
1603 sdev->host->host_no, sdev->channel, sdev->id, sdev->lun);
1604
1605 device_initialize(&sdev->sdev_dev);
1606 sdev->sdev_dev.parent = get_device(&sdev->sdev_gendev);
1607 sdev->sdev_dev.class = &sdev_class;
1608 dev_set_name(&sdev->sdev_dev, "%d:%d:%d:%llu",
1609 sdev->host->host_no, sdev->channel, sdev->id, sdev->lun);
1610 /*
1611 * Get a default scsi_level from the target (derived from sibling
1612 * devices). This is the best we can do for guessing how to set
1613 * sdev->lun_in_cdb for the initial INQUIRY command. For LUN 0 the
1614 * setting doesn't matter, because all the bits are zero anyway.
1615 * But it does matter for higher LUNs.
1616 */
1617 sdev->scsi_level = starget->scsi_level;
1618 if (sdev->scsi_level <= SCSI_2 &&
1619 sdev->scsi_level != SCSI_UNKNOWN &&
1620 !shost->no_scsi2_lun_in_cdb)
1621 sdev->lun_in_cdb = 1;
1622
1623 transport_setup_device(&sdev->sdev_gendev);
1624 spin_lock_irqsave(shost->host_lock, flags);
> 1625 if (sdev->lun_idx != (unsigned long)-1)
1626 WARN_ON(!xa_insert(&starget->devices, sdev->lun_idx,
1627 sdev, GFP_KERNEL));
1628 else {
1629 struct xa_limit scsi_lun_limit = {
1630 .min = 256,
1631 .max = UINT_MAX,
1632 };
1633 WARN_ON(!xa_alloc(&starget->devices, &sdev->lun_idx,
1634 sdev, scsi_lun_limit, GFP_KERNEL));
1635 }
1636 list_add_tail(&sdev->siblings, &shost->__devices);
1637 spin_unlock_irqrestore(shost->host_lock, flags);
1638 /*
1639 * device can now only be removed via __scsi_remove_device() so hold
1640 * the target. Target will be held in CREATED state until something
1641 * beneath it becomes visible (in which case it moves to RUNNING)
1642 */
1643 kref_get(&starget->reap_ref);
1644 }
1645
---
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: 37380 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH 1/4] scsi: convert target lookup to xarray
2020-05-27 14:13 ` [PATCH 1/4] scsi: convert target lookup to xarray Hannes Reinecke
` (2 preceding siblings ...)
2020-05-29 5:01 ` kbuild test robot
@ 2020-05-31 9:10 ` Dan Carpenter
3 siblings, 0 replies; 22+ messages in thread
From: Dan Carpenter @ 2020-05-31 9:10 UTC (permalink / raw)
To: kbuild, Hannes Reinecke, Martin K. Petersen
Cc: lkp, kbuild-all, Christoph Hellwig, Doug Gilbert, Daniel Wagner,
James Bottomley, linux-scsi, Hannes Reinecke
[-- Attachment #1: Type: text/plain, Size: 9488 bytes --]
Hi Hannes,
[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on scsi/for-next v5.7-rc7 next-20200529]
[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/20200527-231824
base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: x86_64-randconfig-m001-20200529 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
drivers/scsi/scsi_scan.c:482 scsi_alloc_target() warn: inconsistent returns '*shost->host_lock'.
drivers/scsi/scsi_scan.c:482 scsi_alloc_target() warn: inconsistent returns 'flags'.
# https://github.com/0day-ci/linux/commit/45b149b239ea9a86968ddbd8ecda1e6c44937b68
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 45b149b239ea9a86968ddbd8ecda1e6c44937b68
vim +482 drivers/scsi/scsi_scan.c
^1da177e4c3f41 Linus Torvalds 2005-04-16 392 static struct scsi_target *scsi_alloc_target(struct device *parent,
^1da177e4c3f41 Linus Torvalds 2005-04-16 393 int channel, uint id)
^1da177e4c3f41 Linus Torvalds 2005-04-16 394 {
^1da177e4c3f41 Linus Torvalds 2005-04-16 395 struct Scsi_Host *shost = dev_to_shost(parent);
^1da177e4c3f41 Linus Torvalds 2005-04-16 396 struct device *dev = NULL;
^1da177e4c3f41 Linus Torvalds 2005-04-16 397 unsigned long flags;
^1da177e4c3f41 Linus Torvalds 2005-04-16 398 const int size = sizeof(struct scsi_target)
^1da177e4c3f41 Linus Torvalds 2005-04-16 399 + shost->transportt->target_size;
5c44cd2afad3f7 James.Smart@Emulex.Com 2005-06-10 400 struct scsi_target *starget;
^1da177e4c3f41 Linus Torvalds 2005-04-16 401 struct scsi_target *found_target;
e63ed0d7a98014 James Bottomley 2014-01-21 402 int error, ref_got;
45b149b239ea9a Hannes Reinecke 2020-05-27 403 unsigned long tid;
^1da177e4c3f41 Linus Torvalds 2005-04-16 404
24669f75a3231f Jes Sorensen 2006-01-16 405 starget = kzalloc(size, GFP_KERNEL);
^1da177e4c3f41 Linus Torvalds 2005-04-16 406 if (!starget) {
cadbd4a5e36dde Harvey Harrison 2008-07-03 407 printk(KERN_ERR "%s: allocation failure\n", __func__);
^1da177e4c3f41 Linus Torvalds 2005-04-16 408 return NULL;
^1da177e4c3f41 Linus Torvalds 2005-04-16 409 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 410 dev = &starget->dev;
^1da177e4c3f41 Linus Torvalds 2005-04-16 411 device_initialize(dev);
e63ed0d7a98014 James Bottomley 2014-01-21 412 kref_init(&starget->reap_ref);
^1da177e4c3f41 Linus Torvalds 2005-04-16 413 dev->parent = get_device(parent);
71610f55fa4db6 Kay Sievers 2008-12-03 414 dev_set_name(dev, "target%d:%d:%d", shost->host_no, channel, id);
b0ed43360fdca2 Hannes Reinecke 2008-03-18 415 dev->bus = &scsi_bus_type;
b0ed43360fdca2 Hannes Reinecke 2008-03-18 416 dev->type = &scsi_target_type;
^1da177e4c3f41 Linus Torvalds 2005-04-16 417 starget->id = id;
^1da177e4c3f41 Linus Torvalds 2005-04-16 418 starget->channel = channel;
f0c0a376d0fcd4 Mike Christie 2008-08-17 419 starget->can_queue = 0;
^1da177e4c3f41 Linus Torvalds 2005-04-16 420 INIT_LIST_HEAD(&starget->devices);
643eb2d932c97a James Bottomley 2008-03-22 421 starget->state = STARGET_CREATED;
7c9d6f16f50d3a Alan Stern 2007-01-08 422 starget->scsi_level = SCSI_2;
c53a284f8be237 Edward Goggin 2009-04-09 423 starget->max_target_blocked = SCSI_DEFAULT_TARGET_BLOCKED;
45b149b239ea9a Hannes Reinecke 2020-05-27 424 tid = scsi_target_index(starget);
ffedb4522571ac James Bottomley 2006-02-23 425 retry:
^1da177e4c3f41 Linus Torvalds 2005-04-16 426 spin_lock_irqsave(shost->host_lock, flags);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
^1da177e4c3f41 Linus Torvalds 2005-04-16 427
45b149b239ea9a Hannes Reinecke 2020-05-27 428 found_target = xa_load(&shost->__targets, tid);
^1da177e4c3f41 Linus Torvalds 2005-04-16 429 if (found_target)
^1da177e4c3f41 Linus Torvalds 2005-04-16 430 goto found;
45b149b239ea9a Hannes Reinecke 2020-05-27 431 if (xa_insert(&shost->__targets, tid, starget, GFP_KERNEL)) {
45b149b239ea9a Hannes Reinecke 2020-05-27 432 dev_printk(KERN_ERR, dev, "target index busy\n");
45b149b239ea9a Hannes Reinecke 2020-05-27 433 kfree(starget);
45b149b239ea9a Hannes Reinecke 2020-05-27 434 return NULL;
^^^^^^^^^^^
Need to drop the lock.
45b149b239ea9a Hannes Reinecke 2020-05-27 435 }
^1da177e4c3f41 Linus Torvalds 2005-04-16 436 spin_unlock_irqrestore(shost->host_lock, flags);
^1da177e4c3f41 Linus Torvalds 2005-04-16 437 /* allocate and add */
a283bd37d00e92 James Bottomley 2005-05-24 438 transport_setup_device(dev);
a283bd37d00e92 James Bottomley 2005-05-24 439 if (shost->hostt->target_alloc) {
32f95792500794 Brian King 2006-02-22 440 error = shost->hostt->target_alloc(starget);
a283bd37d00e92 James Bottomley 2005-05-24 441
a283bd37d00e92 James Bottomley 2005-05-24 442 if(error) {
a283bd37d00e92 James Bottomley 2005-05-24 443 dev_printk(KERN_ERR, dev, "target allocation failed, error %d\n", error);
a283bd37d00e92 James Bottomley 2005-05-24 444 /* don't want scsi_target_reap to do the final
a283bd37d00e92 James Bottomley 2005-05-24 445 * put because it will be under the host lock */
643eb2d932c97a James Bottomley 2008-03-22 446 scsi_target_destroy(starget);
a283bd37d00e92 James Bottomley 2005-05-24 447 return NULL;
a283bd37d00e92 James Bottomley 2005-05-24 448 }
a283bd37d00e92 James Bottomley 2005-05-24 449 }
884d25cc4fda20 James Bottomley 2006-09-05 450 get_device(dev);
a283bd37d00e92 James Bottomley 2005-05-24 451
^1da177e4c3f41 Linus Torvalds 2005-04-16 452 return starget;
^1da177e4c3f41 Linus Torvalds 2005-04-16 453
^1da177e4c3f41 Linus Torvalds 2005-04-16 454 found:
e63ed0d7a98014 James Bottomley 2014-01-21 455 /*
e63ed0d7a98014 James Bottomley 2014-01-21 456 * release routine already fired if kref is zero, so if we can still
e63ed0d7a98014 James Bottomley 2014-01-21 457 * take the reference, the target must be alive. If we can't, it must
e63ed0d7a98014 James Bottomley 2014-01-21 458 * be dying and we need to wait for a new target
e63ed0d7a98014 James Bottomley 2014-01-21 459 */
e63ed0d7a98014 James Bottomley 2014-01-21 460 ref_got = kref_get_unless_zero(&found_target->reap_ref);
e63ed0d7a98014 James Bottomley 2014-01-21 461
^1da177e4c3f41 Linus Torvalds 2005-04-16 462 spin_unlock_irqrestore(shost->host_lock, flags);
e63ed0d7a98014 James Bottomley 2014-01-21 463 if (ref_got) {
12fb8c1574d7d0 Alan Stern 2010-03-18 464 put_device(dev);
^1da177e4c3f41 Linus Torvalds 2005-04-16 465 return found_target;
^1da177e4c3f41 Linus Torvalds 2005-04-16 466 }
e63ed0d7a98014 James Bottomley 2014-01-21 467 /*
e63ed0d7a98014 James Bottomley 2014-01-21 468 * Unfortunately, we found a dying target; need to wait until it's
e63ed0d7a98014 James Bottomley 2014-01-21 469 * dead before we can get a new one. There is an anomaly here. We
e63ed0d7a98014 James Bottomley 2014-01-21 470 * *should* call scsi_target_reap() to balance the kref_get() of the
e63ed0d7a98014 James Bottomley 2014-01-21 471 * reap_ref above. However, since the target being released, it's
e63ed0d7a98014 James Bottomley 2014-01-21 472 * already invisible and the reap_ref is irrelevant. If we call
e63ed0d7a98014 James Bottomley 2014-01-21 473 * scsi_target_reap() we might spuriously do another device_del() on
e63ed0d7a98014 James Bottomley 2014-01-21 474 * an already invisible target.
e63ed0d7a98014 James Bottomley 2014-01-21 475 */
ffedb4522571ac James Bottomley 2006-02-23 476 put_device(&found_target->dev);
e63ed0d7a98014 James Bottomley 2014-01-21 477 /*
e63ed0d7a98014 James Bottomley 2014-01-21 478 * length of time is irrelevant here, we just want to yield the CPU
e63ed0d7a98014 James Bottomley 2014-01-21 479 * for a tick to avoid busy waiting for the target to die.
e63ed0d7a98014 James Bottomley 2014-01-21 480 */
e63ed0d7a98014 James Bottomley 2014-01-21 481 msleep(1);
ffedb4522571ac James Bottomley 2006-02-23 @482 goto retry;
ffedb4522571ac James Bottomley 2006-02-23 483 }
---
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: 37380 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 4/4] scsi: remove direct device lookup per host
2020-05-28 16:36 [PATCHv3 0/4] scsi: use xarray for devices and targets Hannes Reinecke
@ 2020-05-28 16:36 ` Hannes Reinecke
0 siblings, 0 replies; 22+ messages in thread
From: Hannes Reinecke @ 2020-05-28 16:36 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, Doug Gilbert, Daniel Wagner,
Johannes Thumshirn, James Bottomley, linux-scsi, 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 | 67 +++++++++++++++++++++++++++++++++++++++++-----
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, 84 insertions(+), 30 deletions(-)
diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 7109afad0183..004a50a95560 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -382,7 +382,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(&shost->__targets);
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 9dbbc51a1eb5..eac505a169a7 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -554,18 +554,40 @@ 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 (!prev) {
+ starget = xa_find(&shost->__targets, &tid,
+ ULONG_MAX, XA_PRESENT);
+ if (starget) {
+ next = xa_find(&starget->devices, &lun_idx,
+ ULONG_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,
+ ULONG_MAX, XA_PRESENT);
+ if (!next) {
+ /* No more LUNs on this target, switch to the next */
+ lun_idx = 0;
+ starget = xa_find_after(&shost->__targets, &tid,
+ ULONG_MAX, XA_PRESENT);
+ continue;
+ }
if (!scsi_device_get(next))
break;
- next = NULL;
- list = list->next;
}
spin_unlock_irqrestore(shost->host_lock, flags);
@@ -575,6 +597,39 @@ 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,
+ ULONG_MAX, XA_PRESENT);
+ if (starget)
+ return xa_find(&starget->devices, &lun_idx,
+ ULONG_MAX, XA_PRESENT);
+ } 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,
+ ULONG_MAX, XA_PRESENT);
+ if (next)
+ return next;
+ /* No more LUNs on this target, switch to the next */
+ lun_idx = 0;
+ starget = xa_find_after(&shost->__targets, &tid,
+ ULONG_MAX, XA_PRESENT);
+ }
+ 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 c7aba9ba5c0c..e75e7f93f8f6 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);
@@ -1847,17 +1846,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 27c19232f175..63fa57684782 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);
}
@@ -1635,7 +1630,6 @@ void scsi_sysfs_device_initialize(struct scsi_device *sdev)
WARN_ON(xa_alloc(&starget->devices, &sdev->lun_idx,
sdev, scsi_lun_limit, GFP_KERNEL) < 0);
}
- 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 2c6b9d8bc40e..b811b5e3adfe 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. */
@@ -376,6 +373,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
@@ -389,8 +390,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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH 4/4] scsi: remove direct device lookup per host
2020-05-28 8:42 [PATCHv2 0/4] Hannes Reinecke
@ 2020-05-28 8:42 ` Hannes Reinecke
0 siblings, 0 replies; 22+ messages in thread
From: Hannes Reinecke @ 2020-05-28 8:42 UTC (permalink / raw)
To: Martin K. Petersen
Cc: Christoph Hellwig, Doug Gilbert, Johannes Thumshirn,
Daniel Wagner, James Bottomley, linux-scsi, 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 | 67 +++++++++++++++++++++++++++++++++++++++++-----
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, 84 insertions(+), 30 deletions(-)
diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c
index 7109afad0183..004a50a95560 100644
--- a/drivers/scsi/hosts.c
+++ b/drivers/scsi/hosts.c
@@ -382,7 +382,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(&shost->__targets);
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 9dbbc51a1eb5..eac505a169a7 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -554,18 +554,40 @@ 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 (!prev) {
+ starget = xa_find(&shost->__targets, &tid,
+ ULONG_MAX, XA_PRESENT);
+ if (starget) {
+ next = xa_find(&starget->devices, &lun_idx,
+ ULONG_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,
+ ULONG_MAX, XA_PRESENT);
+ if (!next) {
+ /* No more LUNs on this target, switch to the next */
+ lun_idx = 0;
+ starget = xa_find_after(&shost->__targets, &tid,
+ ULONG_MAX, XA_PRESENT);
+ continue;
+ }
if (!scsi_device_get(next))
break;
- next = NULL;
- list = list->next;
}
spin_unlock_irqrestore(shost->host_lock, flags);
@@ -575,6 +597,39 @@ 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,
+ ULONG_MAX, XA_PRESENT);
+ if (starget)
+ return xa_find(&starget->devices, &lun_idx,
+ ULONG_MAX, XA_PRESENT);
+ } 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,
+ ULONG_MAX, XA_PRESENT);
+ if (next)
+ return next;
+ /* No more LUNs on this target, switch to the next */
+ lun_idx = 0;
+ starget = xa_find_after(&shost->__targets, &tid,
+ ULONG_MAX, XA_PRESENT);
+ }
+ 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 c7aba9ba5c0c..e75e7f93f8f6 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);
@@ -1847,17 +1846,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 27c19232f175..63fa57684782 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);
}
@@ -1635,7 +1630,6 @@ void scsi_sysfs_device_initialize(struct scsi_device *sdev)
WARN_ON(xa_alloc(&starget->devices, &sdev->lun_idx,
sdev, scsi_lun_limit, GFP_KERNEL) < 0);
}
- 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 2c6b9d8bc40e..b811b5e3adfe 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. */
@@ -376,6 +373,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
@@ -389,8 +390,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
^ permalink raw reply related [flat|nested] 22+ messages in thread
end of thread, other threads:[~2020-05-31 9:11 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-27 14:13 [RFC PATCH 0/4] scsi: use xarray for devices and targets Hannes Reinecke
2020-05-27 14:13 ` [PATCH 1/4] scsi: convert target lookup to xarray Hannes Reinecke
2020-05-27 14:57 ` Johannes Thumshirn
2020-05-27 15:06 ` Hannes Reinecke
2020-05-28 7:24 ` Daniel Wagner
2020-05-28 7:52 ` Hannes Reinecke
2020-05-28 16:28 ` Bart Van Assche
2020-05-29 5:01 ` kbuild test robot
2020-05-31 9:10 ` Dan Carpenter
2020-05-27 14:13 ` [PATCH 2/4] target_core_pscsi: use __scsi_device_lookup() Hannes Reinecke
2020-05-27 15:05 ` Johannes Thumshirn
2020-05-27 14:13 ` [PATCH 3/4] scsi: move target device list to xarray Hannes Reinecke
2020-05-27 15:34 ` Johannes Thumshirn
2020-05-27 20:13 ` kbuild test robot
2020-05-30 2:47 ` kbuild test robot
2020-05-27 14:14 ` [PATCH 4/4] scsi: remove direct device lookup per host Hannes Reinecke
2020-05-28 8:00 ` kbuild test robot
2020-05-27 16:36 ` [RFC PATCH 0/4] scsi: use xarray for devices and targets Bart Van Assche
2020-05-27 16:59 ` Hannes Reinecke
2020-05-28 3:59 ` Douglas Gilbert
2020-05-28 8:42 [PATCHv2 0/4] Hannes Reinecke
2020-05-28 8:42 ` [PATCH 4/4] scsi: remove direct device lookup per host Hannes Reinecke
2020-05-28 16:36 [PATCHv3 0/4] scsi: use xarray for devices and targets Hannes Reinecke
2020-05-28 16:36 ` [PATCH 4/4] scsi: remove direct device lookup per host Hannes Reinecke
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).