From patchwork Sun May 24 15:58:10 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Douglas Gilbert X-Patchwork-Id: 11567513 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 92FC0912 for ; Sun, 24 May 2020 15:58:27 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 805ED20826 for ; Sun, 24 May 2020 15:58:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2387716AbgEXP61 (ORCPT ); Sun, 24 May 2020 11:58:27 -0400 Received: from smtp.infotech.no ([82.134.31.41]:43420 "EHLO smtp.infotech.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2387636AbgEXP60 (ORCPT ); Sun, 24 May 2020 11:58:26 -0400 Received: from localhost (localhost [127.0.0.1]) by smtp.infotech.no (Postfix) with ESMTP id ABE2E204165; Sun, 24 May 2020 17:58:24 +0200 (CEST) X-Virus-Scanned: by amavisd-new-2.6.6 (20110518) (Debian) at infotech.no Received: from smtp.infotech.no ([127.0.0.1]) by localhost (smtp.infotech.no [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id y6fYri5pUa9C; Sun, 24 May 2020 17:58:22 +0200 (CEST) Received: from xtwo70.bingwo.ca (host-23-251-188-50.dyn.295.ca [23.251.188.50]) by smtp.infotech.no (Postfix) with ESMTPA id 2E8DF204247; Sun, 24 May 2020 17:58:19 +0200 (CEST) From: Douglas Gilbert To: linux-scsi@vger.kernel.org Cc: martin.petersen@oracle.com, jejb@linux.vnet.ibm.com, hare@suse.de Subject: [RFC v2 2/6] scsi: xarray, iterations on scsi_target Date: Sun, 24 May 2020 11:58:10 -0400 Message-Id: <20200524155814.5895-3-dgilbert@interlog.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200524155814.5895-1-dgilbert@interlog.com> References: <20200524155814.5895-1-dgilbert@interlog.com> MIME-Version: 1.0 Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org For reasons unknown, the supplied functions for iterating through all scsi_device objects that belonged to a scsi_target involved going up to the scsi_host object (i.e. the target's parent) and iterating through all scsi_device objects belonging to that scsi_host, and only selecting those scsi_device objects that belonged to the original scsi_target object. While correct, that is very inefficient when it is noted that each scsi_target object has a 'devices' xarray which contains pointers to the scsi_device object it owns. Modify starget_for_each_device() and __starget_for_each_device() to take this more efficient route. Add starg_for_each_device() and __starg_for_each_device() macros in scsi_device.h that are finer grained versions of the existing shost_for_each_device() and __shost_for_each_device() macros. The new __scsi_target_iterate_devices() helper function takes the host_lock to be consistent with the existing __scsi_iterate_devices() function's locking. At this stage the user is _not_ being encouraged to use per scsi_target locks. Signed-off-by: Douglas Gilbert --- drivers/scsi/scsi.c | 68 ++++++++++++++++++++++++++--------- drivers/scsi/scsi_scan.c | 3 +- include/scsi/scsi_device.h | 73 +++++++++++++++++++++++++++++++++++++- 3 files changed, 125 insertions(+), 19 deletions(-) diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c index 61aa68083f67..aa3240f7aed9 100644 --- a/drivers/scsi/scsi.c +++ b/drivers/scsi/scsi.c @@ -588,9 +588,50 @@ struct scsi_device *__scsi_iterate_devices(struct Scsi_Host *shost, } EXPORT_SYMBOL(__scsi_iterate_devices); +/* helper for starg_for_each_device, see that for documentation */ +struct scsi_device *__scsi_target_iterate_devices(struct scsi_target *starg, + struct scsi_device *prev) +{ + struct Scsi_Host *shost = starg_to_shost(starg); + struct scsi_device *next = prev; + unsigned long flags, l_idx; + + if (!shost) + return NULL; + spin_lock_irqsave(shost->host_lock, flags); + if (xa_empty(&starg->devices)) { + next = NULL; + goto unlock; + } + do { + if (!next) { /* get first element iff first iteration */ + l_idx = 0; + next = xa_find(&starg->devices, &l_idx, + scsi_xa_limit_16b.max, XA_PRESENT); + } else { + l_idx = next->sh_idx, + next = xa_find_after(&starg->devices, &l_idx, + scsi_xa_limit_16b.max, + XA_PRESENT); + } + if (next) { + /* skip devices that we can't get a reference to */ + if (!scsi_device_get(next)) + break; + } + } while (next); +unlock: + spin_unlock_irqrestore(shost->host_lock, flags); + + if (prev) + scsi_device_put(prev); + return next; +} +EXPORT_SYMBOL(__scsi_target_iterate_devices); + /** * starget_for_each_device - helper to walk all devices of a target - * @starget: target whose devices we want to iterate over. + * @starg: target whose devices we want to iterate over. * @data: Opaque passed to each function call. * @fn: Function to call on each device * @@ -598,23 +639,20 @@ EXPORT_SYMBOL(__scsi_iterate_devices); * a reference that must be released by scsi_host_put when breaking * out of the loop. */ -void starget_for_each_device(struct scsi_target *starget, void *data, - void (*fn)(struct scsi_device *, void *)) +void starget_for_each_device(struct scsi_target *starg, void *data, + void (*fn)(struct scsi_device *, void *)) { - struct Scsi_Host *shost = starg_to_shost(starget); struct scsi_device *sdev; - shost_for_each_device(sdev, shost) { - if ((sdev->channel == starget->channel) && - (sdev->id == starget->id)) - fn(sdev, data); - } + /* XARRAY: this looks like recursion, but isn't. Rename function? */ + sstarg_for_each_device(sdev, starg) + fn(sdev, data); } EXPORT_SYMBOL(starget_for_each_device); /** * __starget_for_each_device - helper to walk all devices of a target (UNLOCKED) - * @starget: target whose devices we want to iterate over. + * @starg: target whose devices we want to iterate over. * @data: parameter for callback @fn() * @fn: callback function that is invoked for each device * @@ -626,17 +664,13 @@ EXPORT_SYMBOL(starget_for_each_device); * they need to access the device list in irq context. Otherwise you * really want to use starget_for_each_device instead. **/ -void __starget_for_each_device(struct scsi_target *starget, void *data, +void __starget_for_each_device(struct scsi_target *starg, void *data, void (*fn)(struct scsi_device *, void *)) { - struct Scsi_Host *shost = starg_to_shost(starget); struct scsi_device *sdev; - __shost_for_each_device(sdev, shost) { - if ((sdev->channel == starget->channel) && - (sdev->id == starget->id)) - fn(sdev, data); - } + __sstarg_for_each_device(sdev, starg) + fn(sdev, data); } EXPORT_SYMBOL(__starget_for_each_device); diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c index b8f5850c5a04..72a0064a879a 100644 --- a/drivers/scsi/scsi_scan.c +++ b/drivers/scsi/scsi_scan.c @@ -403,7 +403,8 @@ static void scsi_target_reap_ref_put(struct scsi_target *starget) /** * scsi_alloc_target - allocate a new or find an existing target - * @parent: parent of the target (need not be a scsi host) + * @parent: may point to the parent shost, or an intermediate object + * that dev_to_shost() can resolve to the parent shost * @channel: target channel number (zero if no channels) * @id: target id number * diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h index dc9de4cc0e79..4219b8d1b94c 100644 --- a/include/scsi/scsi_device.h +++ b/include/scsi/scsi_device.h @@ -295,6 +295,7 @@ struct scsi_target { * scsi_device.id eventually */ struct xarray devices; /* scsi_device objects owned by this target */ struct device dev; + unsigned int create:1; /* signal that it needs to be added */ unsigned int single_lun:1; /* Indicates we should only * allow I/O to one of the luns @@ -361,6 +362,7 @@ 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 *, u64); +/* XARRAY: these visitor names too close to sstarg_for_each_device macros? */ extern void starget_for_each_device(struct scsi_target *, void *, void (*fn)(struct scsi_device *, void *)); extern void __starget_for_each_device(struct scsi_target *, void *, @@ -370,6 +372,10 @@ extern void __starget_for_each_device(struct scsi_target *, void *, /* only exposed to implement shost_for_each_device */ extern struct scsi_device *__scsi_iterate_devices(struct Scsi_Host *, struct scsi_device *); +/* only exposed to implement starg_for_each_device */ +extern struct scsi_device *__scsi_target_iterate_devices + (struct scsi_target *starg, + struct scsi_device *sdev); /** * shost_for_each_device - iterate over all devices of a host @@ -423,13 +429,78 @@ static inline struct scsi_device *__scsi_h2d_next_it(struct Scsi_Host *shost, * * Note 2: The iteration won't fail but dereferencing sdev might. To access * the xarray features (e.g. marks) associated with sdev safely use: - * xa_for_each(&shost->__devices, l_idx, next) directly then use l_idx. + * xa_for_each(&shost->__devices, l_idx, sdev) directly then use l_idx. */ #define __shost_for_each_device(sdev, shost) \ for ((sdev) = __scsi_h2d_1st_it((shost)); \ (sdev); \ (sdev) = __scsi_h2d_next_it((shost), (sdev))) +/** + * sstarg_for_each_device - iterate over all devices of a target + * @sdev: the &struct scsi_device to use as a cursor + * @starg: the &struct scsi_target to iterate over + * + * Iterator that returns each device attached to @starg. This loop + * takes a reference on each device and releases it at the end. If + * you break out of the loop, you must call scsi_device_put(sdev). + * + * Note: the leading double "ss" is to lessen the similarity between this + * macro and the starget_for_each_device() function declared above. + */ +#define sstarg_for_each_device(sdev, starg) \ + for ((sdev) = __scsi_target_iterate_devices((starg), NULL); \ + (sdev); \ + (sdev) = __scsi_target_iterate_devices((starg), (sdev))) + +/* helper for __sstarg_for_each_device */ +static inline struct scsi_device *__scsi_t2d_1st_it(struct scsi_target *starg) +{ + unsigned long l_idx = 0; + + return xa_find(&starg->devices, &l_idx, scsi_xa_limit_16b.max, + XA_PRESENT); +} + +/* helper for __sstarg_for_each_device */ +static inline struct scsi_device *__scsi_t2d_next_it(struct scsi_target *starg, + struct scsi_device *prev) +{ + unsigned long l_idx; + + if (prev) { + l_idx = prev->starg_idx, + prev = xa_find_after(&starg->devices, &l_idx, + scsi_xa_limit_16b.max, XA_PRESENT); + } + return prev; /* now either _next_ or NULL */ +} + +/** + * __sstarg_for_each_device - iterate over all devices of a target (UNLOCKED) + * @sdev: the &struct scsi_device to use as a cursor + * @starg: the &struct scsi_target to iterate over + * + * Iterator that returns each device attached to @starg. It does _not_ + * take a reference on the scsi_device, so the whole loop must be + * protected by shost->host_lock. + * + * Note: The only reason to use this is because you need to access the + * device list in interrupt context. Otherwise you really want to use + * starg_for_each_device instead. + * + * Note 2: The iteration won't fail but dereferencing sdev might. To access + * the xarray features (e.g. marks) associated with sdev safely use: + * xa_for_each(&starg->devices, l_idx, sdev) directly then use l_idx. + * + * Note 3: the leading double "ss" is to lessen the similarity between this + * macro and the __starget_for_each_device() function declared above. + */ +#define __sstarg_for_each_device(sdev, starg) \ + for ((sdev) = __scsi_t2d_1st_it((starg)); \ + (sdev); \ + (sdev) = __scsi_t2d_next_it((starg), (sdev))) + extern int scsi_change_queue_depth(struct scsi_device *, int); extern int scsi_track_queue_full(struct scsi_device *, int); From patchwork Sun May 24 15:58:11 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Douglas Gilbert X-Patchwork-Id: 11567517 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 2A39F912 for ; Sun, 24 May 2020 15:58:30 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 17AFB20826 for ; Sun, 24 May 2020 15:58:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2387732AbgEXP62 (ORCPT ); Sun, 24 May 2020 11:58:28 -0400 Received: from smtp.infotech.no ([82.134.31.41]:43426 "EHLO smtp.infotech.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2387625AbgEXP62 (ORCPT ); Sun, 24 May 2020 11:58:28 -0400 Received: from localhost (localhost [127.0.0.1]) by smtp.infotech.no (Postfix) with ESMTP id 22F4B204247; Sun, 24 May 2020 17:58:26 +0200 (CEST) X-Virus-Scanned: by amavisd-new-2.6.6 (20110518) (Debian) at infotech.no Received: from smtp.infotech.no ([127.0.0.1]) by localhost (smtp.infotech.no [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id JIqwamFnw1wN; Sun, 24 May 2020 17:58:23 +0200 (CEST) Received: from xtwo70.bingwo.ca (host-23-251-188-50.dyn.295.ca [23.251.188.50]) by smtp.infotech.no (Postfix) with ESMTPA id 3E1D720424C; Sun, 24 May 2020 17:58:20 +0200 (CEST) From: Douglas Gilbert To: linux-scsi@vger.kernel.org Cc: martin.petersen@oracle.com, jejb@linux.vnet.ibm.com, hare@suse.de Subject: [RFC v2 3/6] scsi: xarray mark sdev_del state Date: Sun, 24 May 2020 11:58:11 -0400 Message-Id: <20200524155814.5895-4-dgilbert@interlog.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200524155814.5895-1-dgilbert@interlog.com> References: <20200524155814.5895-1-dgilbert@interlog.com> MIME-Version: 1.0 Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org One of the features of an xarray is the ability to mark the pointers that it holds. Sadly there are only two marks available per xarray so it is not possible to map marks to the sdev_state enumeration which holds 9 states. Since several loops skip sdev_s in SDEV_DEL state then its negation was chosen as one mark: SCSI_XA_NON_SDEV_DEL. To support this mark a new iterator macro: shost_for_each_non_del_device(sdev, shost) has been introduced plus a helper function: __scsi_iterate_non_del_devices(shost, sdev). The new iterator macro is used once (in scsi_scan.c) and the new mark is used directly in three other loops. The machinery to support the mark is placed in a helper (scsi_adjust_non_sdev_del_mark()) called mainly by scsi_device_set_state(). Other locations that set sdev_state were checked and some additional calls to that helper were added. Following a complaint from 'make W=1' the formal name of a parameter to both starget_for_each_device() and __starget_for_each_device() was changed from starg to starget. Signed-off-by: Douglas Gilbert --- drivers/scsi/scsi.c | 49 +++++++++++++++++++++++++++++++++----- drivers/scsi/scsi_lib.c | 34 ++++++++++++++++++++++++-- drivers/scsi/scsi_scan.c | 14 +++++------ drivers/scsi/scsi_sysfs.c | 19 +++++++++------ include/scsi/scsi_device.h | 19 +++++++++++++++ include/scsi/scsi_host.h | 2 ++ 6 files changed, 114 insertions(+), 23 deletions(-) diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c index aa3240f7aed9..0fb650aebcfb 100644 --- a/drivers/scsi/scsi.c +++ b/drivers/scsi/scsi.c @@ -588,6 +588,45 @@ struct scsi_device *__scsi_iterate_devices(struct Scsi_Host *shost, } EXPORT_SYMBOL(__scsi_iterate_devices); +/* helper for shost_for_each_non_del_device, see that for documentation */ +struct scsi_device *__scsi_iterate_non_del_devices(struct Scsi_Host *shost, + struct scsi_device *prev) +{ + struct scsi_device *next = prev; + unsigned long flags, l_idx; + + spin_lock_irqsave(shost->host_lock, flags); + if (xa_empty(&shost->__devices)) { + next = NULL; + goto unlock; + } + do { + if (!next) { /* get first element iff first iteration */ + l_idx = 0; + next = xa_find(&shost->__devices, &l_idx, + scsi_xa_limit_16b.max, + SCSI_XA_NON_SDEV_DEL); + } else { + l_idx = next->sh_idx, + next = xa_find_after(&shost->__devices, &l_idx, + scsi_xa_limit_16b.max, + SCSI_XA_NON_SDEV_DEL); + } + if (next) { + /* skip devices that we can't get a reference to */ + if (!scsi_device_get(next)) + break; + } + } while (next); +unlock: + spin_unlock_irqrestore(shost->host_lock, flags); + + if (prev) + scsi_device_put(prev); + return next; +} +EXPORT_SYMBOL(__scsi_iterate_non_del_devices); + /* helper for starg_for_each_device, see that for documentation */ struct scsi_device *__scsi_target_iterate_devices(struct scsi_target *starg, struct scsi_device *prev) @@ -695,9 +734,8 @@ struct scsi_device *__scsi_device_lookup_by_target(struct scsi_target *starget, unsigned long l_idx; struct scsi_device *sdev; - xa_for_each(&starget->devices, l_idx, sdev) { - if (sdev->sdev_state == SDEV_DEL) - continue; + xa_for_each_marked(&starget->devices, l_idx, sdev, + SCSI_XA_NON_SDEV_DEL) { if (sdev->lun ==lun) return sdev; } @@ -754,9 +792,8 @@ struct scsi_device *__scsi_device_lookup(struct Scsi_Host *shost, unsigned long l_idx; struct scsi_device *sdev; - xa_for_each(&shost->__devices, l_idx, sdev) { - if (sdev->sdev_state == SDEV_DEL) - continue; + xa_for_each_marked(&shost->__devices, l_idx, sdev, + SCSI_XA_NON_SDEV_DEL) { if (sdev->channel == channel && sdev->id == id && sdev->lun ==lun) return sdev; diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 68df68f54fc8..f484bf2b5d7d 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -2217,6 +2217,27 @@ scsi_test_unit_ready(struct scsi_device *sdev, int timeout, int retries, } EXPORT_SYMBOL(scsi_test_unit_ready); +/* Assume host_lock taken and that xahp->xa_lock is the same lock. */ +static inline void +scsi_adjust_non_sdev_del_mark(struct scsi_device *sdev, + enum scsi_device_state old_state, + enum scsi_device_state new_state) +{ + struct xarray *xatp = + &to_scsi_target(sdev->sdev_gendev.parent)->devices; + struct xarray *xahp = &sdev->host->__devices; + + if (old_state == new_state) + return; + if (old_state == SDEV_DEL) { + xa_set_mark(xatp, sdev->starg_idx, SCSI_XA_NON_SDEV_DEL); + xa_set_mark(xahp, sdev->sh_idx, SCSI_XA_NON_SDEV_DEL); + } else if (new_state == SDEV_DEL) { + xa_clear_mark(xatp, sdev->starg_idx, SCSI_XA_NON_SDEV_DEL); + xa_clear_mark(xahp, sdev->sh_idx, SCSI_XA_NON_SDEV_DEL); + } +} + /** * scsi_device_set_state - Take the given device through the device state model. * @sdev: scsi device to change the state of. @@ -2330,6 +2351,8 @@ scsi_device_set_state(struct scsi_device *sdev, enum scsi_device_state state) } sdev->offline_already = false; + if (unlikely(oldstate == SDEV_DEL || state == SDEV_DEL)) + scsi_adjust_non_sdev_del_mark(sdev, oldstate, state); sdev->sdev_state = state; return 0; @@ -2731,14 +2754,21 @@ int scsi_internal_device_unblock_nowait(struct scsi_device *sdev, switch (sdev->sdev_state) { case SDEV_BLOCK: case SDEV_TRANSPORT_OFFLINE: + if (unlikely(new_state == SDEV_DEL)) + scsi_adjust_non_sdev_del_mark(sdev, sdev->sdev_state, + SDEV_DEL); sdev->sdev_state = new_state; break; case SDEV_CREATED_BLOCK: if (new_state == SDEV_TRANSPORT_OFFLINE || - new_state == SDEV_OFFLINE) + new_state == SDEV_OFFLINE) { sdev->sdev_state = new_state; - else + } else { + if (unlikely(new_state == SDEV_DEL)) + scsi_adjust_non_sdev_del_mark + (sdev, sdev->sdev_state, SDEV_DEL); sdev->sdev_state = SDEV_CREATED; + } break; case SDEV_CANCEL: case SDEV_OFFLINE: diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c index 72a0064a879a..f5a5e48ca5ae 100644 --- a/drivers/scsi/scsi_scan.c +++ b/drivers/scsi/scsi_scan.c @@ -280,7 +280,7 @@ static struct scsi_device *scsi_alloc_sdev(struct scsi_target *starget, scsi_change_queue_depth(sdev, sdev->host->cmd_per_lun ? sdev->host->cmd_per_lun : 1); - scsi_sysfs_device_initialize(sdev); + scsi_sysfs_device_initialize(sdev); /* marked as non_sdev_del */ if (shost->hostt->slave_alloc) { ret = shost->hostt->slave_alloc(sdev); @@ -1708,10 +1708,9 @@ int scsi_scan_host_selected(struct Scsi_Host *shost, unsigned int channel, static void scsi_sysfs_add_devices(struct Scsi_Host *shost) { struct scsi_device *sdev; - shost_for_each_device(sdev, shost) { - /* target removed before the device could be added */ - if (sdev->sdev_state == SDEV_DEL) - continue; + + /* target may be removed before devices could be added */ + shost_for_each_non_del_device(sdev, shost) { /* If device is already visible, skip adding it to sysfs */ if (sdev->is_visible) continue; @@ -1883,9 +1882,8 @@ void scsi_forget_host(struct Scsi_Host *shost) restart: spin_lock_irqsave(shost->host_lock, flags); - xa_for_each(&shost->__devices, l_idx, sdev) { - if (sdev->sdev_state == SDEV_DEL) - continue; + xa_for_each_marked(&shost->__devices, l_idx, sdev, + SCSI_XA_NON_SDEV_DEL) { spin_unlock_irqrestore(shost->host_lock, flags); __scsi_remove_device(sdev); goto restart; diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index 4bfcf33139a2..6fdd4648f374 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -1481,15 +1481,15 @@ static void __scsi_remove_target(struct scsi_target *starget) spin_lock_irqsave(shost->host_lock, flags); restart: - xa_for_each(&starget->devices, l_idx, sdev) { + xa_for_each_marked(&starget->devices, l_idx, sdev, + SCSI_XA_NON_SDEV_DEL) { /* * 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->sdev_state == SDEV_DEL || - sdev->sdev_state == SDEV_CANCEL || + if (sdev->sdev_state == SDEV_CANCEL || !get_device(&sdev->sdev_gendev)) continue; spin_unlock_irqrestore(shost->host_lock, flags); @@ -1621,16 +1621,21 @@ void scsi_sysfs_device_initialize(struct scsi_device *sdev) /* XARRAY: might add in hole */ res = xa_alloc(&starget->devices, &u_idx, sdev, scsi_xa_limit_16b, GFP_ATOMIC); - if (res < 0) + if (res < 0) { pr_err("%s: xa_alloc 1 failure errno=%d\n", __func__, -res); - else + } else { sdev->starg_idx = u_idx; + xa_set_mark(&starget->devices, u_idx, SCSI_XA_NON_SDEV_DEL); + } res = xa_alloc(&shost->__devices, &u_idx, sdev, scsi_xa_limit_16b, GFP_ATOMIC); - if (res < 0) + if (res < 0) { pr_err("%s: xa_alloc 2 failure errno=%d\n", __func__, -res); - else + } else { sdev->sh_idx = u_idx; + /* already have host_lock which is shost->__devices.xa_lock */ + xa_set_mark(&shost->__devices, u_idx, SCSI_XA_NON_SDEV_DEL); + } 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 4219b8d1b94c..23bf686cbabc 100644 --- a/include/scsi/scsi_device.h +++ b/include/scsi/scsi_device.h @@ -372,6 +372,9 @@ extern void __starget_for_each_device(struct scsi_target *, void *, /* only exposed to implement shost_for_each_device */ extern struct scsi_device *__scsi_iterate_devices(struct Scsi_Host *, struct scsi_device *); +/* only exposed to implement shost_for_each_non_del_device */ +extern struct scsi_device *__scsi_iterate_non_del_devices + (struct Scsi_Host *shost, struct scsi_device *sdev); /* only exposed to implement starg_for_each_device */ extern struct scsi_device *__scsi_target_iterate_devices (struct scsi_target *starg, @@ -391,6 +394,22 @@ extern struct scsi_device *__scsi_target_iterate_devices (sdev); \ (sdev) = __scsi_iterate_devices((shost), (sdev))) +/** + * shost_for_each_non_del_device - iterate over those scsi_devices that do not + * have the SDEV_DEL state and are associated + * with given host + * @sdev: the &struct scsi_device to use as a cursor + * @shost: the &struct Scsi_Host to iterate over + * + * Iterator that returns each device attached to @shost. This loop + * takes a reference on each device and releases it at the end. If + * you break out of the loop, you must call scsi_device_put(sdev). + */ +#define shost_for_each_non_del_device(sdev, shost) \ + for ((sdev) = __scsi_iterate_non_del_devices((shost), NULL); \ + (sdev); \ + (sdev) = __scsi_iterate_non_del_devices((shost), (sdev))) + /* helper for __shost_for_each_device */ static inline struct scsi_device *__scsi_h2d_1st_it(struct Scsi_Host *shost) { diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h index e6386f3d6de1..0e94b1feb8e9 100644 --- a/include/scsi/scsi_host.h +++ b/include/scsi/scsi_host.h @@ -33,6 +33,8 @@ struct scsi_transport_template; /* XARRAY: this limits number of devices (LUs) in host to 64K */ #define scsi_xa_limit_16b XA_LIMIT(0, USHRT_MAX) +#define SCSI_XA_NON_SDEV_DEL XA_MARK_1 + struct scsi_host_template { struct module *module; const char *name; From patchwork Sun May 24 15:58:12 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Douglas Gilbert X-Patchwork-Id: 11567515 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id E4C16912 for ; Sun, 24 May 2020 15:58:28 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id CDA5420826 for ; Sun, 24 May 2020 15:58:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2387725AbgEXP62 (ORCPT ); Sun, 24 May 2020 11:58:28 -0400 Received: from smtp.infotech.no ([82.134.31.41]:43434 "EHLO smtp.infotech.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2387703AbgEXP62 (ORCPT ); Sun, 24 May 2020 11:58:28 -0400 Received: from localhost (localhost [127.0.0.1]) by smtp.infotech.no (Postfix) with ESMTP id 2A01420423F; Sun, 24 May 2020 17:58:26 +0200 (CEST) X-Virus-Scanned: by amavisd-new-2.6.6 (20110518) (Debian) at infotech.no Received: from smtp.infotech.no ([127.0.0.1]) by localhost (smtp.infotech.no [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id feIX13gAInlH; Sun, 24 May 2020 17:58:24 +0200 (CEST) Received: from xtwo70.bingwo.ca (host-23-251-188-50.dyn.295.ca [23.251.188.50]) by smtp.infotech.no (Postfix) with ESMTPA id 4E3E8204255; Sun, 24 May 2020 17:58:21 +0200 (CEST) From: Douglas Gilbert To: linux-scsi@vger.kernel.org Cc: martin.petersen@oracle.com, jejb@linux.vnet.ibm.com, hare@suse.de Subject: [RFC v2 4/6] scsi: improve scsi_device_lookup Date: Sun, 24 May 2020 11:58:12 -0400 Message-Id: <20200524155814.5895-5-dgilbert@interlog.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200524155814.5895-1-dgilbert@interlog.com> References: <20200524155814.5895-1-dgilbert@interlog.com> MIME-Version: 1.0 Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org When the __scsi_device_lookup() function is given a "ctl" (i.e. the latter part of "hctl" tuple), improve the loop to find a matching device (LU) in the given host. Rather than loop over all LUs in the host, first loop over all targets to find a match on "ct" then, if found, loop over all LUs in that target for a match on "l". These nested loops are better since they don't visit LUs belonging to non-matching targets. This improvement flows through to the locked version of this function, namely scsi_device_lookup(). Remove a 21 year old comment by the author that no longer seem relevant. Signed-off-by: Douglas Gilbert --- drivers/scsi/scsi.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c index 0fb650aebcfb..9e7658aebdb7 100644 --- a/drivers/scsi/scsi.c +++ b/drivers/scsi/scsi.c @@ -35,7 +35,6 @@ * * Jiffies wrap fixes (host->resetting), 3 Dec 1998 Andrea Arcangeli * - * out_of_space hacks, D. Gilbert (dpg) 990608 */ #include @@ -789,16 +788,31 @@ EXPORT_SYMBOL(scsi_device_lookup_by_target); struct scsi_device *__scsi_device_lookup(struct Scsi_Host *shost, uint channel, uint id, u64 lun) { - unsigned long l_idx; + unsigned long l_idx, m_idx; + struct scsi_target *starg; struct scsi_device *sdev; - xa_for_each_marked(&shost->__devices, l_idx, sdev, + if (xa_empty(&shost->__devices)) + return NULL; + if (xa_empty(&shost->__targets)) + goto inconsistent; + xa_for_each(&shost->__targets, l_idx, starg) { + if (!(starg->id == id && starg->channel == channel)) + continue; + xa_for_each_marked(&starg->devices, m_idx, sdev, + SCSI_XA_NON_SDEV_DEL) { + if (sdev->lun == lun) + return sdev; + } + } + return NULL; +inconsistent: + xa_for_each_marked(&shost->__devices, m_idx, sdev, SCSI_XA_NON_SDEV_DEL) { - if (sdev->channel == channel && sdev->id == id && - sdev->lun ==lun) + if (sdev->id == id && sdev->channel == channel && + sdev->lun == lun) return sdev; } - return NULL; } EXPORT_SYMBOL(__scsi_device_lookup); From patchwork Sun May 24 15:58:13 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Douglas Gilbert X-Patchwork-Id: 11567521 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 1CBEC912 for ; Sun, 24 May 2020 15:58:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 0FEAB207DA for ; Sun, 24 May 2020 15:58:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2387740AbgEXP6a (ORCPT ); Sun, 24 May 2020 11:58:30 -0400 Received: from smtp.infotech.no ([82.134.31.41]:43443 "EHLO smtp.infotech.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2387703AbgEXP6a (ORCPT ); Sun, 24 May 2020 11:58:30 -0400 Received: from localhost (localhost [127.0.0.1]) by smtp.infotech.no (Postfix) with ESMTP id 31D4D20423B; Sun, 24 May 2020 17:58:28 +0200 (CEST) X-Virus-Scanned: by amavisd-new-2.6.6 (20110518) (Debian) at infotech.no Received: from smtp.infotech.no ([127.0.0.1]) by localhost (smtp.infotech.no [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Yj9iyJmeklmU; Sun, 24 May 2020 17:58:26 +0200 (CEST) Received: from xtwo70.bingwo.ca (host-23-251-188-50.dyn.295.ca [23.251.188.50]) by smtp.infotech.no (Postfix) with ESMTPA id 5EC27204258; Sun, 24 May 2020 17:58:22 +0200 (CEST) From: Douglas Gilbert To: linux-scsi@vger.kernel.org Cc: martin.petersen@oracle.com, jejb@linux.vnet.ibm.com, hare@suse.de Subject: [RFC v2 5/6] scsi: add __scsi_target_lookup Date: Sun, 24 May 2020 11:58:13 -0400 Message-Id: <20200524155814.5895-6-dgilbert@interlog.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200524155814.5895-1-dgilbert@interlog.com> References: <20200524155814.5895-1-dgilbert@interlog.com> MIME-Version: 1.0 Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org Hides the xarray based lookup of a target given the channel and target identifiers within a shost. Use this and __sstarg_for_each_device() where useful. Break the pattern when deleting to jump back and restart the iteration as that seems to be a linked list quirk. May change semantics if elements can be added in the newly created holes in the xarray at the same time as the ongoing deletion. Signed-off-by: Douglas Gilbert --- drivers/scsi/scsi.c | 24 ++++++++++++++++++++++++ drivers/scsi/scsi_error.c | 34 +++++++++++++++++----------------- drivers/scsi/scsi_lib.c | 8 +++----- drivers/scsi/scsi_scan.c | 4 ++-- drivers/scsi/scsi_sysfs.c | 8 +++----- include/scsi/scsi_device.h | 2 ++ 6 files changed, 51 insertions(+), 29 deletions(-) diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c index 9e7658aebdb7..1b1fc8d4e5c2 100644 --- a/drivers/scsi/scsi.c +++ b/drivers/scsi/scsi.c @@ -667,6 +667,30 @@ struct scsi_device *__scsi_target_iterate_devices(struct scsi_target *starg, } EXPORT_SYMBOL(__scsi_target_iterate_devices); +/** + * __scsi_target_lookup - helper to find target given channel and target_id + * @shost: host to search within + * @chan: channel number + * @t_id: target identifier + * + * No locks or references taken. Returns NULL if not found. + */ +struct scsi_target *__scsi_target_lookup(struct Scsi_Host *shost, + uint chan, uint t_id) +{ + unsigned long l_idx; + struct scsi_target *starg = NULL; + + if (shost) { + xa_for_each(&shost->__targets, l_idx, starg) { + if (chan == starg->channel && t_id == starg->id) + break; + } + } + return starg; +} +EXPORT_SYMBOL(__scsi_target_lookup); + /** * starget_for_each_device - helper to walk all devices of a target * @starg: target whose devices we want to iterate over. diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index 978be1602f71..b2dbfa5f73a0 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -644,6 +644,7 @@ static void scsi_handle_queue_ramp_up(struct scsi_device *sdev) { struct scsi_host_template *sht = sdev->host->hostt; struct scsi_device *tmp_sdev; + struct scsi_target *starg = scsi_target(sdev); if (!sht->track_queue_depth || sdev->queue_depth >= sdev->max_queue_depth) @@ -658,13 +659,10 @@ static void scsi_handle_queue_ramp_up(struct scsi_device *sdev) return; /* - * Walk all devices of a target and do - * ramp up on them. + * Walk all devices of a target and do ramp up on them. */ - shost_for_each_device(tmp_sdev, sdev->host) { - if (tmp_sdev->channel != sdev->channel || - tmp_sdev->id != sdev->id || - tmp_sdev->queue_depth == sdev->max_queue_depth) + sstarg_for_each_device(tmp_sdev, starg) { + if (tmp_sdev->queue_depth == sdev->max_queue_depth) continue; scsi_change_queue_depth(tmp_sdev, tmp_sdev->queue_depth + 1); @@ -672,25 +670,26 @@ static void scsi_handle_queue_ramp_up(struct scsi_device *sdev) } } +/* + * queue_full corresponds to the SCSI status: "Task Set Full". Its scope is + * at the SCSI taret device level. + */ static void scsi_handle_queue_full(struct scsi_device *sdev) { struct scsi_host_template *sht = sdev->host->hostt; struct scsi_device *tmp_sdev; + struct scsi_target *starg = scsi_target(sdev); if (!sht->track_queue_depth) return; - shost_for_each_device(tmp_sdev, sdev->host) { - if (tmp_sdev->channel != sdev->channel || - tmp_sdev->id != sdev->id) - continue; + sstarg_for_each_device(tmp_sdev, starg) /* * We do not know the number of commands that were at - * the device when we got the queue full so we start + * the target when we got the queue full so we start * from the highest possible value and work our way down. */ scsi_track_queue_full(tmp_sdev, tmp_sdev->queue_depth - 1); - } } /** @@ -2305,12 +2304,13 @@ EXPORT_SYMBOL(scsi_report_bus_reset); void scsi_report_device_reset(struct Scsi_Host *shost, int channel, int target) { struct scsi_device *sdev; + struct scsi_target *starg = __scsi_target_lookup(shost, channel, + target); - __shost_for_each_device(sdev, shost) { - if (channel == sdev_channel(sdev) && - target == sdev_id(sdev)) - __scsi_report_device_reset(sdev, NULL); - } + if (!starg) + return; + __sstarg_for_each_device(sdev, starg) + __scsi_report_device_reset(sdev, NULL); } EXPORT_SYMBOL(scsi_report_device_reset); diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index f484bf2b5d7d..034ec2e57d1c 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -374,7 +374,7 @@ static void scsi_single_lun_run(struct scsi_device *current_sdev) struct Scsi_Host *shost = current_sdev->host; struct scsi_device *sdev; struct scsi_target *starget = scsi_target(current_sdev); - unsigned long flags, l_idx; + unsigned long flags; spin_lock_irqsave(shost->host_lock, flags); starget->starget_sdev_user = NULL; @@ -392,7 +392,7 @@ static void scsi_single_lun_run(struct scsi_device *current_sdev) if (starget->starget_sdev_user) goto out; /* XARRAY: was list_for_each_entry_safe(); is this safe ? */ - xa_for_each(&starget->devices, l_idx, sdev) { + __sstarg_for_each_device(sdev, starget) { if (sdev == current_sdev) continue; if (scsi_device_get(sdev)) @@ -2217,14 +2217,12 @@ scsi_test_unit_ready(struct scsi_device *sdev, int timeout, int retries, } EXPORT_SYMBOL(scsi_test_unit_ready); -/* Assume host_lock taken and that xahp->xa_lock is the same lock. */ static inline void scsi_adjust_non_sdev_del_mark(struct scsi_device *sdev, enum scsi_device_state old_state, enum scsi_device_state new_state) { - struct xarray *xatp = - &to_scsi_target(sdev->sdev_gendev.parent)->devices; + struct xarray *xatp = &scsi_target(sdev)->devices; struct xarray *xahp = &sdev->host->__devices; if (old_state == new_state) diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c index f5a5e48ca5ae..c055ee083ea9 100644 --- a/drivers/scsi/scsi_scan.c +++ b/drivers/scsi/scsi_scan.c @@ -1880,13 +1880,13 @@ void scsi_forget_host(struct Scsi_Host *shost) struct scsi_device *sdev; unsigned long flags, l_idx; - restart: spin_lock_irqsave(shost->host_lock, flags); xa_for_each_marked(&shost->__devices, l_idx, sdev, SCSI_XA_NON_SDEV_DEL) { spin_unlock_irqrestore(shost->host_lock, flags); __scsi_remove_device(sdev); - goto restart; + /* XARRAY: was a goto to before first line */ + 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 6fdd4648f374..394230d69afd 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -1480,7 +1480,6 @@ static void __scsi_remove_target(struct scsi_target *starget) unsigned long flags, l_idx; spin_lock_irqsave(shost->host_lock, flags); - restart: xa_for_each_marked(&starget->devices, l_idx, sdev, SCSI_XA_NON_SDEV_DEL) { /* @@ -1496,8 +1495,7 @@ 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); - /* XARRAY: is this goto start of loop correct ? */ - goto restart; + /* XARRAY: was a goto to restart loop */ } spin_unlock_irqrestore(shost->host_lock, flags); } @@ -1516,7 +1514,6 @@ void scsi_remove_target(struct device *dev) struct scsi_target *starget; unsigned long flags, l_idx; -restart: spin_lock_irqsave(shost->host_lock, flags); xa_for_each(&shost->__targets, l_idx, starget) { if (starget->state == STARGET_DEL || @@ -1532,7 +1529,8 @@ void scsi_remove_target(struct device *dev) spin_unlock_irqrestore(shost->host_lock, flags); __scsi_remove_target(starget); scsi_target_reap(starget); - goto restart; + /* XARRAY: was a goto to before first line */ + spin_lock_irqsave(shost->host_lock, flags); } } spin_unlock_irqrestore(shost->host_lock, flags); diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h index 23bf686cbabc..e019621416d5 100644 --- a/include/scsi/scsi_device.h +++ b/include/scsi/scsi_device.h @@ -362,6 +362,8 @@ 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 *, u64); +extern struct scsi_target *__scsi_target_lookup(struct Scsi_Host *shost, + uint chan, uint t_id); /* XARRAY: these visitor names too close to sstarg_for_each_device macros? */ extern void starget_for_each_device(struct scsi_target *, void *, void (*fn)(struct scsi_device *, void *)); From patchwork Sun May 24 15:58:14 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Douglas Gilbert X-Patchwork-Id: 11567519 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 8F86F159A for ; Sun, 24 May 2020 15:58:30 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 827C820826 for ; Sun, 24 May 2020 15:58:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2387737AbgEXP6a (ORCPT ); Sun, 24 May 2020 11:58:30 -0400 Received: from smtp.infotech.no ([82.134.31.41]:43438 "EHLO smtp.infotech.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2387636AbgEXP63 (ORCPT ); Sun, 24 May 2020 11:58:29 -0400 Received: from localhost (localhost [127.0.0.1]) by smtp.infotech.no (Postfix) with ESMTP id 9DB42204248; Sun, 24 May 2020 17:58:27 +0200 (CEST) X-Virus-Scanned: by amavisd-new-2.6.6 (20110518) (Debian) at infotech.no Received: from smtp.infotech.no ([127.0.0.1]) by localhost (smtp.infotech.no [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Ayn6TEaUUNRT; Sun, 24 May 2020 17:58:26 +0200 (CEST) Received: from xtwo70.bingwo.ca (host-23-251-188-50.dyn.295.ca [23.251.188.50]) by smtp.infotech.no (Postfix) with ESMTPA id 7255020423B; Sun, 24 May 2020 17:58:23 +0200 (CEST) From: Douglas Gilbert To: linux-scsi@vger.kernel.org Cc: martin.petersen@oracle.com, jejb@linux.vnet.ibm.com, hare@suse.de Subject: [RFC v2 6/6] scsi: count number of targets Date: Sun, 24 May 2020 11:58:14 -0400 Message-Id: <20200524155814.5895-7-dgilbert@interlog.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200524155814.5895-1-dgilbert@interlog.com> References: <20200524155814.5895-1-dgilbert@interlog.com> MIME-Version: 1.0 Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org If less than 2 used to flatten iteration in __scsi_device_lookup(). The majority of hosts will have 1 target and 1 LU (device). Signed-off-by: Douglas Gilbert --- drivers/scsi/scsi.c | 12 ++++-------- drivers/scsi/scsi_scan.c | 5 ++++- include/scsi/scsi_host.h | 1 + 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c index 1b1fc8d4e5c2..6acd85ce21dd 100644 --- a/drivers/scsi/scsi.c +++ b/drivers/scsi/scsi.c @@ -557,10 +557,6 @@ struct scsi_device *__scsi_iterate_devices(struct Scsi_Host *shost, unsigned long flags, l_idx; spin_lock_irqsave(shost->host_lock, flags); - if (xa_empty(&shost->__devices)) { - next = NULL; - goto unlock; - } do { if (!next) { /* get first element iff first iteration */ l_idx = 0; @@ -578,7 +574,6 @@ struct scsi_device *__scsi_iterate_devices(struct Scsi_Host *shost, break; } } while (next); -unlock: spin_unlock_irqrestore(shost->host_lock, flags); if (prev) @@ -818,8 +813,9 @@ struct scsi_device *__scsi_device_lookup(struct Scsi_Host *shost, if (xa_empty(&shost->__devices)) return NULL; - if (xa_empty(&shost->__targets)) - goto inconsistent; + if (shost->num_targets < 2) + goto flat_iteration; + xa_for_each(&shost->__targets, l_idx, starg) { if (!(starg->id == id && starg->channel == channel)) continue; @@ -830,7 +826,7 @@ struct scsi_device *__scsi_device_lookup(struct Scsi_Host *shost, } } return NULL; -inconsistent: +flat_iteration: xa_for_each_marked(&shost->__devices, m_idx, sdev, SCSI_XA_NON_SDEV_DEL) { if (sdev->id == id && sdev->channel == channel && diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c index c055ee083ea9..d665da2b01a4 100644 --- a/drivers/scsi/scsi_scan.c +++ b/drivers/scsi/scsi_scan.c @@ -320,7 +320,9 @@ static void scsi_target_destroy(struct scsi_target *starget) shost->hostt->target_destroy(starget); /* XARRAY: was list_del_init(); why the _init ? */ e_starg = xa_erase(&shost->__targets, starget->sh_idx); - if (e_starg != starget) + if (e_starg == starget) + --shost->num_targets; + else pr_err("%s: bad xa_erase()\n", __func__); spin_unlock_irqrestore(shost->host_lock, flags); put_device(dev); @@ -465,6 +467,7 @@ static struct scsi_target *scsi_alloc_target(struct device *parent, return NULL; } starget->sh_idx = u_idx; + ++shost->num_targets; spin_unlock_irqrestore(shost->host_lock, flags); /* allocate and add */ transport_setup_device(dev); diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h index 0e94b1feb8e9..f49298a4c452 100644 --- a/include/scsi/scsi_host.h +++ b/include/scsi/scsi_host.h @@ -528,6 +528,7 @@ struct Scsi_Host { */ struct xarray __devices; /* array of scsi_debug objs */ struct xarray __targets; /* array of scsi_target objs */ + int num_targets; /* modified under host_lock */ struct list_head starved_list;