Re: [PATCH] SCSI: don't get target/host busy_count in scsi_mq_get_budget()

Bart Van Assche
2017-11-06T19:45:27Z

On Sat, 2017-11-04 at 08:19 -0600, Jens Axboe wrote:
> On 11/03/2017 07:55 PM, Ming Lei wrote:
> > It is very expensive to atomic_inc/atomic_dec the host wide counter of
> > host->busy_count, and it should have been avoided via blk-mq's mechanism
> > of getting driver tag, which uses the more efficient way of sbitmap queue.
> >
> > Also we don't check atomic_read(&sdev->device_busy) in scsi_mq_get_budget()
> > and don't run queue if the counter becomes zero, so IO hang may be caused
> > if all requests are completed just before the current SCSI device
> > is added to shost->starved_list.
>
> This looks like an improvement. I have added it for 4.15.
>
> Bart, does this fix your hang?

No, it doesn't. After I had reduced starget->can_queue in the SRP initiator I
ran into the following hang while running the srp-test software:

sysrq: SysRq : Show Blocked State
task PC stack pid father
systemd-udevd D 0 19882 467 0x80000106
Call Trace:
__schedule+0x2fa/0xbb0
schedule+0x36/0x90
io_schedule+0x16/0x40
__lock_page+0x10a/0x140
truncate_inode_pages_range+0x4ff/0x800
truncate_inode_pages+0x15/0x20
kill_bdev+0x35/0x40
__blkdev_put+0x6d/0x1f0
blkdev_put+0x4e/0x130
blkdev_close+0x25/0x30
__fput+0xed/0x1f0
____fput+0xe/0x10
task_work_run+0x8b/0xc0
do_exit+0x38d/0xc70
do_group_exit+0x50/0xd0
get_signal+0x2ad/0x8c0
do_signal+0x28/0x680
exit_to_usermode_loop+0x5a/0xa0
do_syscall_64+0x12e/0x170
entry_SYSCALL64_slow_path+0x25/0x25

The SRP initiator driver was modified as follows for this test:

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index a6664467651e..9d24a871cc2e 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c

@@ -2835,6 +2839,13 @@ static int srp_reset_host(struct scsi_cmnd *scmnd)
return srp_reconnect_rport(target->rport) == 0 ? SUCCESS : FAILED;
}

+static int srp_target_alloc(struct scsi_target *starget)
+{
+ starget->can_queue = 1;
+ return 0;
+}
+
static int srp_slave_alloc(struct scsi_device *sdev)
{
struct Scsi_Host *shost = sdev->host;
@@ -3039,6 +3050,7 @@ static struct scsi_host_template srp_template = {
.module = THIS_MODULE,
.name = "InfiniBand SRP initiator",
.proc_name = DRV_NAME,
+ .target_alloc = srp_target_alloc,
.slave_alloc = srp_slave_alloc,
.slave_configure = srp_slave_configure,
.info = srp_target_info,

Bart.