On Tue, Jan 23, 2018 at 08:17:02AM -0800, Bart Van Assche wrote:

On 01/22/18 16:57, Ming Lei wrote:

quoted

Even though RCU lock is held during dispatch, preemption or interrupt
can happen too, so it is simply wrong to depend on the timing to make
sure __blk_mq_run_hw_queue() can see the request in this situation.

It is very unlikely that this race will ever be hit because that race exists
for less than one microsecond and the smallest timeout that can be specified
for delayed queue rerunning is one millisecond. Let's address this race if
anyone ever finds a way to hit it.

Please don't depend on the timing which is often fragile, as we can make it
correct in a generic approach. Also we should avoid to make every driver to
follow this way for dealing with most of STS_RESOURCE, right?

quoted

quoted

quoted

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index d9ca1dfab154..55be2550c555 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c

@@ -2030,9 +2030,9 @@ static blk_status_t scsi_queue_rq(struct blk_mq_hw_ctx *hctx,
  	case BLK_STS_OK:
  		break;
  	case BLK_STS_RESOURCE:
-		if (atomic_read(&sdev->device_busy) == 0 &&
-		    !scsi_device_blocked(sdev))
-			blk_mq_delay_run_hw_queue(hctx, SCSI_QUEUE_DELAY);
+		if (atomic_read(&sdev->device_busy) ||
+		    scsi_device_blocked(sdev))
+			ret = BLK_STS_DEV_RESOURCE;
  		break;
  	default:
  		/*

The above introduces two changes that have not been mentioned in the
description of this patch:
- The queue rerunning delay is changed from 3 ms into 10 ms. Where is the
   explanation of this change? Does this change have a positive or negative
   performance impact?

How can that be a issue for SCSI? The rerunning delay is only triggered
when there isn't any in-flight requests in SCSI queue.

That change will result in more scsi_queue_rq() calls and hence in higher
CPU usage. By how much the CPU usage is increased will depend on the CPU
time required by the LLD .queuecommand() callback if that function gets
invoked.

No, this patch won't increase CPU usage on SCSI, and the only change is to move
the blk_mq_delay_run_hw_queue() out of SCSI's .queue_rq(), and the delay
becomes 10.

Thanks,
Ming