Re: [PATCH RFC] scsi: core: remove unsed 'restarts' from scsi_device

From: Yu Kuai
Date: Sun Nov 27 2022 - 22:35:27 EST




在 2022/11/28 11:27, Ming Lei 写道:
On Sat, Nov 26, 2022 at 04:54:46PM +0800, Yu Kuai wrote:
Hi,

在 2022/11/18 19:30, Yu Kuai 写道:
From: Yu Kuai <yukuai3@xxxxxxxxxx>

During code review, I found that 'restarts' is not useful anymore after
the following commits:

1) commit ab3cee3762e5 ("blk-mq: In blk_mq_dispatch_rq_list() "no budget"
is a reason to kick")
2) commit d3b38596875d ("blk-mq: run queue no matter whether the request
is the last request")
3) commit 673235f91531 ("scsi: core: Fix race between handling STS_RESOURCE
and completion")

Now that if get budget ever failed, block layer will make sure to
trigger new run queue for the hctx. Hence there is no need to run queue
from scsi layer in this case.


But scsi_run_queue_async() needs to run all hw queue because budget is
actually LUN/request queue wide.

Why the hw queue need to run if get budget never failed in this hw
queue?



Does anyone has suggestions about this patch?

More info why I tried to remove this:

while testing megaraid with 4 nvme with none elevator, the default
queue_depth is 128, while I test it with fio 128 jobs and 1 iodepth,
bw is about 4Gib/s, however, if I test with 128 jobs and 2 iodepth,
bw is decreased to about 0.8Gib/s, and with this patch applied,
bw can stay 4Gib/s in the later case.

What is .can_queue and nr_hw_queues in your setting?
test cmd:
fio -name=0 -ioengine=libaio -direct=1 -group_reporting=1 -randseed=2022 -rwmixread=70 -refill_buffers -filename=/dev/sdg -numjobs=128 -size=1TB -runtime=60s -bs=4k -iodepth=2 -rw=randwrite

test environment:
arm64 Kunpeng-920, 128 cpu
megaraid with 4 NVMEs, 128 hctx and queue_depth is 128




thanks,
Ming

.