Do not requeue requests if FAILFAST is set

Hi James,

the FAILFAST flag is currently not honored in scsi_request_fn(). So if scsi_dispatch_cmd() returns an error or the queue is not ready, the request will always be requeued regardless of the FAILFAST setting.
This can result in a delayed fallback for multipathing, as these requests will only be catched once the timeout expires.

This patch avoids requeuing if the FAILFAST flag is set, and terminates the request immediately.

Comments et al welcome.

Cheers,

Hannes

Dr. Hannes Reinecke		      zSeries & Storage
[email protected]			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: Markus Rex, HRB 16746 (AG Nürnberg)

Do not requeue requests if REQ_FAILFAST is set

Any requests with the REQ_FAILFAST flag set should not be requeued to the requeust queue, but rather terminated directly. Otherwise the multipath failover will stall until the command timeout triggers.

index 277f1b6..a12d2f1 100644
@@ -1200,6 +1200,11 @@ static int scsi_prep_fn(struct request_queue *q, struct request *req)
 			 */
 			if (!(req->cmd_flags & REQ_PREEMPT))
 				ret = BLKPREP_DEFER;
+			/*
+			 * Return failfast requests immediately
+			 */
+			if (req->cmd_flags & REQ_FAILFAST)
+				ret = BLKPREP_KILL;
 			break;
 		default:
 			/*

@@ -1327,6 +1332,17 @@ static inline int scsi_host_queue_ready(struct request_queue *q,
 	return 1;
 }
 
+static void __scsi_kill_request(struct request *req)
+{
+	struct scsi_cmnd *cmd = req->special;
+	struct scsi_device *sdev = cmd->device;
+
+	cmd->result = DID_NO_CONNECT << 16;
+	atomic_inc(&cmd->device->iorequest_cnt);
+	sdev->device_busy--;
+	__scsi_done(cmd);
+}
+
 /*
  * Kill a request for a dead device
  */

@@ -1440,8 +1456,16 @@ static void scsi_request_fn(struct request_queue *q)
 		 * accept it.
 		 */
 		req = elv_next_request(q);
-		if (!req || !scsi_dev_queue_ready(q, sdev))
+		if (!req)
+			break;
+
+		if (!scsi_dev_queue_ready(q, sdev)) {
+			if (req->cmd_flags & REQ_FAILFAST) {
+				scsi_kill_request(req, q);
+				continue;
+			}
 			break;
+		}
 
 		if (unlikely(!scsi_device_online(sdev))) {
 			sdev_printk(KERN_ERR, sdev,

@@ -1522,8 +1546,12 @@ static void scsi_request_fn(struct request_queue *q)
 	 * later time.
 	 */
 	spin_lock_irq(q->queue_lock);
-	blk_requeue_request(q, req);
-	sdev->device_busy--;
+	if (unlikely(req->cmd_flags & REQ_FAILFAST))
+		__scsi_kill_request(req);
+	else {
+		blk_requeue_request(q, req);
+		sdev->device_busy--;
+	}
 	if(sdev->device_busy == 0)
 		blk_plug_device(q);
  out:

I think that might be better than my transport errors patch. If multipath is being used then we told people to set the recovery timeout, to a very low value. So that way commands would not sit in a blocked queue very long.

With your patch, if the queue was blocked because we are doing recovery, then the command is returned upwards as soon as the driver/class returns the command to the scsi layer. And that is nice since it fixes that issue where the fibre channel class/driver fails commands in the driver when fail fast tmo fires, but commands in the blocked queue sit around until dev loss tmo fires or the rport comes back.

Is there any case though where fail fast is set on the request, but the driver really does want to retry on the driver? Was there some other user of something like DID_IMM_RETRY that could be used with multipath or raid and when they return DID_IMM_RETRY they wanted to bypass the fail fast checks? If so that would be my only reason for splitting up the error values and relying on the transport class timers (fc fail fast tmo and iscsi recovery tmo) to decide when to fail commands (assuming that the fc class is also changed so when fail fast tmo fires blocked commands are failed).

Oh yeah since we are now doing dm-multipath with SAS, what is that class doing as far as failing commands when there is a transport issue? Does it have something like fc's fail fast tmo? Does it fail running commands right away so the scsi layer can requeue or does it try to do some lower level recovery and try to continue the command until some timeout fires?