diff mbox series

scsi: Avoid unnecessary iterations on __scsi_scan_target

Message ID 38cee464-7320-87a9-f55c-f0db4679fc0a@oracle.com
State New
Headers show
Series scsi: Avoid unnecessary iterations on __scsi_scan_target | expand

Commit Message

Anjali Kulkarni June 26, 2020, 11:53 p.m. UTC
There is a loop in scsi_scan_channel(), which calls
__scsi_scan_target() 255 times, even when it has found a target/device on a
lun in the first iteration; this ends up adding a 2 secs delay to the boot
time. The for loop in scsi_scan_channel() adding 2 secs to boot time is as
follows:

for (id = 0; id < shost->max_id; ++id) {
     ...
     __scsi_scan_target(&shost->shost_gendev, channel,
                               order_id, lun, rescan);
}

__scsi_scan_target() calls scsi_probe_and_add_lun() which calls
scsi_probe_lun(), hence scsi_probe_lun() ends up getting called 255 times.
Each call of scsi_probe_lun() takes 0.007865 secs.
0.007865 multiplied by 255 = 2.00557 secs.
By adding a break in above for loop when a valid device on lun is found,
we can avoid this 2 secs delay improving boot time by 2 secs.

The flow of code is depicted in the following sequence of events:

do_scan_async() ->
     do_scsi_scan_host()->
         scsi_scan_host_selected() ->
             scsi_scan_channel()
                 __scsi_scan_target() -> this is called shost->max_id times
                                         (255 times)
                     scsi_probe_and_add_lun-> this is called 255 times
                         scsi_probe_lun : called 255 times, each call
                                          takes 0.007865 secs.

The with break and without break boot time differences can be seen with
systemd-analyze:

With break:

[root@vm-sec root]# systemd-analyze
Startup finished in 2.038s (kernel) + 1.210s (initrd) + 14.198s 
(userspace) = 17.446s

Without break:

[root@vm-sec root]# systemd-analyze
Startup finished in 2.036s (kernel) + 3.342s (initrd) + 14.255s 
(userspace) = 19.634s

Signed-off-by: Anjali Kulkarni <anjali.k.kulkarni@oracle.com>
---
  drivers/scsi/scsi_scan.c | 17 +++++++++++------
  1 file changed, 11 insertions(+), 6 deletions(-)


@@ -1578,6 +1581,7 @@ static void __scsi_scan_target(struct device 
*parent, unsigned int channel,
      scsi_target_reap(starget);

      put_device(&starget->dev);
+    return res;
  }

  /**
@@ -1646,8 +1650,9 @@ static void scsi_scan_channel(struct Scsi_Host 
*shost, unsigned int channel,
                  order_id = shost->max_id - id - 1;
              else
                  order_id = id;
-            __scsi_scan_target(&shost->shost_gendev, channel,
-                    order_id, lun, rescan);
+            if (__scsi_scan_target(&shost->shost_gendev, channel,
+                order_id, lun, rescan) != SCSI_SCAN_NO_RESPONSE)
+                break;
          }
      else
          __scsi_scan_target(&shost->shost_gendev, channel,
diff mbox series

Patch

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index f2437a7570ce..a519b944b512 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -1527,23 +1527,23 @@  void scsi_rescan_device(struct device *dev)
  }
  EXPORT_SYMBOL(scsi_rescan_device);

-static void __scsi_scan_target(struct device *parent, unsigned int channel,
+static int __scsi_scan_target(struct device *parent, unsigned int channel,
          unsigned int id, u64 lun, enum scsi_scan_mode rescan)
  {
      struct Scsi_Host *shost = dev_to_shost(parent);
      blist_flags_t bflags = 0;
-    int res;
+    int res = SCSI_SCAN_NO_RESPONSE;
      struct scsi_target *starget;

      if (shost->this_id == id)
          /*
           * Don't scan the host adapter
           */
-        return;
+        return res;

      starget = scsi_alloc_target(parent, channel, id);
      if (!starget)
-        return;
+        return res;
      scsi_autopm_get_target(starget);

      if (lun != SCAN_WILD_CARD) {
@@ -1551,6 +1551,9 @@  static void __scsi_scan_target(struct device 
*parent, unsigned int channel,
           * Scan for a specific host/chan/id/lun.
           */
          scsi_probe_and_add_lun(starget, lun, NULL, NULL, rescan, NULL);
+        res = scsi_probe_and_add_lun(starget, lun, NULL, NULL,
+                rescan, NULL);
+
          goto out_reap;
      }