block: move SCSI timeout check into block

PATCH In review James Bottomley 2 participants 3 messages Dec 16, 2008 – Dec 17, 2008

Subsystems (2)

SCSI Block layer

lore

Share & tools

lore

James Bottomley

author maintainer

2008-12-16 23:21 UTC #0 lore raw

PATCH 1/1 block: move SCSI timeout check into block

We can eliminate the SCSI command timed out check entirely if the block layer does this for us. The way to do this in block is to check how long the request has been outstanding if a requeue is requested and ending it if we've gone over retries * timeout.

This will also eliminate many cases in SCSI where we evade the command timeout for various reasons (like initial success converted to requeue)

James

---

block/blk-core.c +13 -1

index c36aa98..d32af31 100644
@@ -926,6 +926,8 @@ EXPORT_SYMBOL(blk_start_queueing);
  */
 void blk_requeue_request(struct request_queue *q, struct request *rq)
 {
+	unsigned long wait_for = (rq->retries + 1) * rq->timeout;
+
 	blk_delete_timer(rq);
 	blk_clear_rq_complete(rq);
 	blk_add_trace_rq(q, rq, BLK_TA_REQUEUE);

@@ -933,7 +935,17 @@ void blk_requeue_request(struct request_queue *q, struct request *rq)
 	if (blk_rq_tagged(rq))
 		blk_queue_end_tag(q, rq);
 
-	elv_requeue_request(q, rq);
+	if (time_before(rq->start_time + wait_for, jiffies)) {
+		int bytes = (rq->hard_nr_sectors << 9);
+
+		printk(KERN_ERR "%s: timing out command, waited %lus\n",
+		       rq->rq_disk ? rq->rq_disk->disk_name : "?",
+		       wait_for/HZ);
+		if (blk_pc_request(rq))
+			bytes = rq->data_len;
+		blk_end_request(rq, -EIO, bytes);
+	} else
+		elv_requeue_request(q, rq);
 }
 EXPORT_SYMBOL(blk_requeue_request);

Jens Axboe

2008-12-17 08:06 UTC #1 lore raw

Quoted 26 lines · James Bottomley

We can eliminate the SCSI command timed out check entirely if the block layer does this for us. The way to do this in block is to check how long the request has been outstanding if a requeue is requested and ending it if we've gone over retries * timeout.
This will also eliminate many cases in SCSI where we evade the command timeout for various reasons (like initial success converted to requeue)
James
---
block/blk-core.c +2 -0
index c36aa98..d32af31 100644
@@ -926,6 +926,8 @@ EXPORT_SYMBOL(blk_start_queueing);
  */
 void blk_requeue_request(struct request_queue *q, struct request *rq)
 {
+	unsigned long wait_for = (rq->retries + 1) * rq->timeout;
+
 	blk_delete_timer(rq);
 	blk_clear_rq_complete(rq);
 	blk_add_trace_rq(q, rq, BLK_TA_REQUEUE);

block/blk-core.c void blk_requeue_request(struct request_queue *q, struct request *rq) · line 938

Quoted 5 lines

block/blk-core.c +1 -1

@@ -933,7 +935,17 @@ void blk_requeue_request(struct request_queue *q, struct request *rq)
 	if (blk_rq_tagged(rq))
 		blk_queue_end_tag(q, rq);
 
-	elv_requeue_request(q, rq);
+	if (time_before(rq->start_time + wait_for, jiffies)) {

I prefer time_after(), reads better.

block/blk-core.c void blk_requeue_request(struct request_queue *q, struct request *rq) · line 946

Quoted 8 lines

block/blk-core.c +8 -0

+		int bytes = (rq->hard_nr_sectors << 9);
+
+		printk(KERN_ERR "%s: timing out command, waited %lus\n",
+		       rq->rq_disk ? rq->rq_disk->disk_name : "?",
+		       wait_for/HZ);
+		if (blk_pc_request(rq))
+			bytes = rq->data_len;
+		blk_end_request(rq, -EIO, bytes);

You can just do

                blk_end_request(rq, -EIO, blk_rq_bytes(rq));

and get rid of the 'bytes' variable and type check.

block/blk-core.c void blk_requeue_request(struct request_queue *q, struct request *rq) · line 950

Quoted 4 lines

block/blk-core.c +2 -0

+	} else
+		elv_requeue_request(q, rq);
 }
 EXPORT_SYMBOL(blk_requeue_request);

But I agree with the patch. Will you rediff, or shall I just apply with the suggest changes?

Signature · 1 line

Jens Axboe

James Bottomley

author maintainer

2008-12-17 16:06 UTC #2 lore raw

Quoted 50 lines · Jens Axboe

Quoted 26 lines · James Bottomley
We can eliminate the SCSI command timed out check entirely if the block layer does this for us. The way to do this in block is to check how long the request has been outstanding if a requeue is requested and ending it if we've gone over retries * timeout.
This will also eliminate many cases in SCSI where we evade the command timeout for various reasons (like initial success converted to requeue)
James
---
block/blk-core.c +2 -0
index c36aa98..d32af31 100644
@@ -926,6 +926,8 @@ EXPORT_SYMBOL(blk_start_queueing);
  */
 void blk_requeue_request(struct request_queue *q, struct request *rq)
 {
+	unsigned long wait_for = (rq->retries + 1) * rq->timeout;
+
 	blk_delete_timer(rq);
 	blk_clear_rq_complete(rq);
 	blk_add_trace_rq(q, rq, BLK_TA_REQUEUE);
block/blk-core.c void blk_requeue_request(struct request_queue *q, struct request *rq) · line 938
Quoted 5 lines
block/blk-core.c +1 -1
@@ -933,7 +935,17 @@ void blk_requeue_request(struct request_queue *q, struct request *rq)
 	if (blk_rq_tagged(rq))
 		blk_queue_end_tag(q, rq);
 
-	elv_requeue_request(q, rq);
+	if (time_before(rq->start_time + wait_for, jiffies)) {
I prefer time_after(), reads better.
block/blk-core.c void blk_requeue_request(struct request_queue *q, struct request *rq) · line 946
Quoted 8 lines
block/blk-core.c +8 -0
+		int bytes = (rq->hard_nr_sectors << 9);
+
+		printk(KERN_ERR "%s: timing out command, waited %lus\n",
+		       rq->rq_disk ? rq->rq_disk->disk_name : "?",
+		       wait_for/HZ);
+		if (blk_pc_request(rq))
+			bytes = rq->data_len;
+		blk_end_request(rq, -EIO, bytes);
You can just do
                blk_end_request(rq, -EIO, blk_rq_bytes(rq));
and get rid of the 'bytes' variable and type check.

Quoted 7 lines

block/blk-core.c void blk_requeue_request(struct request_queue *q, struct request *rq) · line 950
Quoted 4 lines
block/blk-core.c +2 -0
+	} else
+		elv_requeue_request(q, rq);
 }
 EXPORT_SYMBOL(blk_requeue_request);
But I agree with the patch. Will you rediff, or shall I just apply with the suggest changes?

I'll rediff ... we're going to have to test it out quite a bit, especially when I pull the SCSI timeout stuff.

James