* [PATCH] scsi: Avoid unnecessary iterations on __scsi_scan_target
@ 2020-06-26 23:53 Anjali Kulkarni
2020-06-27 0:03 ` James Bottomley
0 siblings, 1 reply; 4+ messages in thread
From: Anjali Kulkarni @ 2020-06-26 23:53 UTC (permalink / raw)
To: martin.petersen; +Cc: jejb, linux-scsi, Anjali Kulkarni
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(-)
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;
}
@@ -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,
--
2.26.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] scsi: Avoid unnecessary iterations on __scsi_scan_target
2020-06-26 23:53 [PATCH] scsi: Avoid unnecessary iterations on __scsi_scan_target Anjali Kulkarni
@ 2020-06-27 0:03 ` James Bottomley
2020-06-27 0:17 ` Anjali Kulkarni
0 siblings, 1 reply; 4+ messages in thread
From: James Bottomley @ 2020-06-27 0:03 UTC (permalink / raw)
To: Anjali Kulkarni, martin.petersen; +Cc: linux-scsi
On Fri, 2020-06-26 at 16:53 -0700, Anjali Kulkarni wrote:
> 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.
What HBA is this? This code is for legacy scanning of busses which
require it, which is pretty much only SPI. The max_id of even the
latest SPI bus should only be 16, so where is 255 coming from?
James
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] scsi: Avoid unnecessary iterations on __scsi_scan_target
2020-06-27 0:03 ` James Bottomley
@ 2020-06-27 0:17 ` Anjali Kulkarni
2020-06-27 0:21 ` James Bottomley
0 siblings, 1 reply; 4+ messages in thread
From: Anjali Kulkarni @ 2020-06-27 0:17 UTC (permalink / raw)
To: jejb, martin.petersen; +Cc: linux-scsi
Thanks James.
This is on a VM on OCI cloud, using virtio scsi.
Here is output of lspci on the VM:
00:00.0 Host bridge: Intel Corporation 440FX - 82441FX PMC [Natoma] (rev 02)
00:01.0 ISA bridge: Intel Corporation 82371SB PIIX3 ISA [Natoma/Triton II]
00:01.1 IDE interface: Intel Corporation 82371SB PIIX3 IDE
[Natoma/Triton II]
00:01.2 USB controller: Intel Corporation 82371SB PIIX3 USB
[Natoma/Triton II] (rev 01)
00:01.3 Bridge: Intel Corporation 82371AB/EB/MB PIIX4 ACPI (rev 03)
00:02.0 VGA compatible controller: Device 1234:1111 (rev 02)
00:03.0 Ethernet controller: Red Hat, Inc. Virtio network device
00:04.0 SCSI storage controller: Red Hat, Inc. Virtio SCSI
With 16 iterations, we could reduce time by 0.13 secs.
Anjali
On 6/26/20 17:03, James Bottomley wrote:
> On Fri, 2020-06-26 at 16:53 -0700, Anjali Kulkarni wrote:
>> 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.
> What HBA is this? This code is for legacy scanning of busses which
> require it, which is pretty much only SPI. The max_id of even the
> latest SPI bus should only be 16, so where is 255 coming from?
>
> James
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] scsi: Avoid unnecessary iterations on __scsi_scan_target
2020-06-27 0:17 ` Anjali Kulkarni
@ 2020-06-27 0:21 ` James Bottomley
0 siblings, 0 replies; 4+ messages in thread
From: James Bottomley @ 2020-06-27 0:21 UTC (permalink / raw)
To: Anjali Kulkarni, martin.petersen; +Cc: linux-scsi
On Fri, 2020-06-26 at 17:17 -0700, Anjali Kulkarni wrote:
> Thanks James.
>
> This is on a VM on OCI cloud, using virtio scsi.
>
> Here is output of lspci on the VM:
>
> 00:00.0 Host bridge: Intel Corporation 440FX - 82441FX PMC [Natoma]
> (rev 02)
> 00:01.0 ISA bridge: Intel Corporation 82371SB PIIX3 ISA
> [Natoma/Triton II]
> 00:01.1 IDE interface: Intel Corporation 82371SB PIIX3 IDE
> [Natoma/Triton II]
> 00:01.2 USB controller: Intel Corporation 82371SB PIIX3 USB
> [Natoma/Triton II] (rev 01)
> 00:01.3 Bridge: Intel Corporation 82371AB/EB/MB PIIX4 ACPI (rev 03)
> 00:02.0 VGA compatible controller: Device 1234:1111 (rev 02)
> 00:03.0 Ethernet controller: Red Hat, Inc. Virtio network device
> 00:04.0 SCSI storage controller: Red Hat, Inc. Virtio SCSI
I think the right thing to do would be to fix virtio-SCSI. It already
has the guest to host communication to know how many targets are
configured. There's shouldn't be any need to do scanning.
James
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-06-27 0:22 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-26 23:53 [PATCH] scsi: Avoid unnecessary iterations on __scsi_scan_target Anjali Kulkarni
2020-06-27 0:03 ` James Bottomley
2020-06-27 0:17 ` Anjali Kulkarni
2020-06-27 0:21 ` James Bottomley
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).