| Message ID | 20200529134730.146573-6-hare@suse.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | scsi: use xarray for devices and targets | expand |
Hi Hannes,
I love your patch! Perhaps something to improve:
[auto build test WARNING on mkp-scsi/for-next]
[also build test WARNING on scsi/for-next v5.7-rc7 next-20200529]
[cannot apply to hch-configfs/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Hannes-Reinecke/scsi-use-xarray-for-devices-and-targets/20200531-083913
base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
config: x86_64-allyesconfig (attached as .config)
compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project 2388a096e7865c043e83ece4e26654bd3d1a20d5)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install x86_64 cross compiling tool for clang build
# apt-get install binutils-x86-64-linux-gnu
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>
All warnings (new ones prefixed by >>, old ones prefixed by <<):
>> drivers/scsi/scsi_error.c:686:15: warning: variable 'starget' is uninitialized when used here [-Wuninitialized]
xa_for_each(&starget->__devices, lun_idx, tmp_sdev) {
^~~~~~~
include/linux/xarray.h:497:20: note: expanded from macro 'xa_for_each'
xa_for_each_start(xa, index, entry, 0)
^~
include/linux/xarray.h:473:20: note: expanded from macro 'xa_for_each_start'
xa_for_each_range(xa, index, entry, start, ULONG_MAX)
^~
include/linux/xarray.h:445:23: note: expanded from macro 'xa_for_each_range'
entry = xa_find(xa, &index, last, XA_PRESENT); ^~
drivers/scsi/scsi_error.c:679:29: note: initialize the variable 'starget' to silence this warning
struct scsi_target *starget;
^
= NULL
1 warning generated.
vim +/starget +686 drivers/scsi/scsi_error.c
675
676 static void scsi_handle_queue_full(struct scsi_device *sdev)
677 {
678 struct scsi_host_template *sht = sdev->host->hostt;
679 struct scsi_target *starget;
680 struct scsi_device *tmp_sdev;
681 unsigned long lun_idx = 0;
682
683 if (!sht->track_queue_depth)
684 return;
685
> 686 xa_for_each(&starget->__devices, lun_idx, tmp_sdev) {
687 if (tmp_sdev->sdev_state == SDEV_DEL)
688 continue;
689 /*
690 * We do not know the number of commands that were at
691 * the device when we got the queue full so we start
692 * from the highest possible value and work our way down.
693 */
694 scsi_track_queue_full(tmp_sdev, tmp_sdev->queue_depth - 1);
695 }
696 }
697
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c index 6a1d8c6bd8f9..296cecd61d3a 100644 --- a/drivers/scsi/scsi.c +++ b/drivers/scsi/scsi.c @@ -652,7 +652,7 @@ EXPORT_SYMBOL(__scsi_iterate_devices_unlocked); * @id: ID of the target * */ -static struct scsi_target *__scsi_target_lookup(struct Scsi_Host *shost, +struct scsi_target *__scsi_target_lookup(struct Scsi_Host *shost, u16 channel, u16 id) { return xa_load(&shost->__targets, (channel << 16) | id); diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index 978be1602f71..f13789e86601 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -643,7 +643,9 @@ EXPORT_SYMBOL_GPL(scsi_check_sense); static void scsi_handle_queue_ramp_up(struct scsi_device *sdev) { struct scsi_host_template *sht = sdev->host->hostt; + struct scsi_target *starget; struct scsi_device *tmp_sdev; + unsigned long lun_idx = 0; if (!sht->track_queue_depth || sdev->queue_depth >= sdev->max_queue_depth) @@ -661,10 +663,9 @@ static void scsi_handle_queue_ramp_up(struct scsi_device *sdev) * Walk all devices of a target and do * ramp up on them. */ - shost_for_each_device(tmp_sdev, sdev->host) { - if (tmp_sdev->channel != sdev->channel || - tmp_sdev->id != sdev->id || - tmp_sdev->queue_depth == sdev->max_queue_depth) + starget = __scsi_target_lookup(sdev->host, sdev->channel, sdev->id); + xa_for_each(&starget->__devices, lun_idx, tmp_sdev) { + if (tmp_sdev->queue_depth == sdev->max_queue_depth) continue; scsi_change_queue_depth(tmp_sdev, tmp_sdev->queue_depth + 1); @@ -675,14 +676,15 @@ static void scsi_handle_queue_ramp_up(struct scsi_device *sdev) static void scsi_handle_queue_full(struct scsi_device *sdev) { struct scsi_host_template *sht = sdev->host->hostt; + struct scsi_target *starget; struct scsi_device *tmp_sdev; + unsigned long lun_idx = 0; if (!sht->track_queue_depth) return; - shost_for_each_device(tmp_sdev, sdev->host) { - if (tmp_sdev->channel != sdev->channel || - tmp_sdev->id != sdev->id) + xa_for_each(&starget->__devices, lun_idx, tmp_sdev) { + if (tmp_sdev->sdev_state == SDEV_DEL) continue; /* * We do not know the number of commands that were at @@ -2271,10 +2273,16 @@ int scsi_error_handler(void *data) */ void scsi_report_bus_reset(struct Scsi_Host *shost, int channel) { + struct scsi_target *starget; struct scsi_device *sdev; + unsigned long tid = 0; - __shost_for_each_device(sdev, shost) { - if (channel == sdev_channel(sdev)) + xa_for_each(&shost->__targets, tid, starget) { + unsigned long lun_idx = 0; + + if (starget->channel != channel) + continue; + xa_for_each(&starget->__devices, lun_idx, sdev) __scsi_report_device_reset(sdev, NULL); } } @@ -2304,13 +2312,14 @@ EXPORT_SYMBOL(scsi_report_bus_reset); */ void scsi_report_device_reset(struct Scsi_Host *shost, int channel, int target) { + struct scsi_target *starget; struct scsi_device *sdev; + unsigned long lun_idx = 0; - __shost_for_each_device(sdev, shost) { - if (channel == sdev_channel(sdev) && - target == sdev_id(sdev)) + starget = __scsi_target_lookup(shost, channel, target); + if (starget) + xa_for_each(&starget->__devices, lun_idx, sdev) __scsi_report_device_reset(sdev, NULL); - } } EXPORT_SYMBOL(scsi_report_device_reset); diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h index 22b6585e28b4..0a87c95359aa 100644 --- a/drivers/scsi/scsi_priv.h +++ b/drivers/scsi/scsi_priv.h @@ -49,6 +49,8 @@ enum scsi_devinfo_key { SCSI_DEVINFO_SPI, }; +extern struct scsi_target *__scsi_target_lookup(struct Scsi_Host *shost, + u16 channel, u16 id); extern blist_flags_t scsi_get_device_flags(struct scsi_device *sdev, const unsigned char *vendor, const unsigned char *model);
For SCSI EH most shost_for_each_sdev() calls are just to filter out devices for specific targets or channels. These calls can be made more efficient using direct xarray lookup and iterators. Signed-off-by: Hannes Reinecke <hare@suse.de> --- drivers/scsi/scsi.c | 2 +- drivers/scsi/scsi_error.c | 35 ++++++++++++++++++++++------------- drivers/scsi/scsi_priv.h | 2 ++ 3 files changed, 25 insertions(+), 14 deletions(-)