scsi tcq fixes for 2.6.27

scsi tcq fixes for 2.6.27

[michaelc]

The following patches were made over scsi-misc. They are bug fixes, but
I am not sure if they are critical enough send to linus for 2.6.26.
They fix a bug where we can send a device too much IO, because we were
setting the queue depth to the driver's can_queue. I had thought that
we could end up with IO timeouts and queue fulls, but I have not seen
any reports on the lists, so people are probably not running IO work
loads on these drivers.

[PATCH 1/3] scsi: fix shared tag map setup

[michaelc]

From: Mike Christie <michaelc@cs.wisc.edu>

Currently qla4xxx and stex pass in their can_queue values into
scsi_activate_tcq because they wanted the tag map that large.
The problem with this is that it ends up also setting the queue
depth to that large value. All we want to do this in this case
is set the device queue depth and the other device settings.
We do not need to touch the tag map sizing because the drivers
had setup that map according to their can_queue limits when the
shared map was created.

The scsi mid layer in request_fn will then handle the case where we
have more requests than available tags when it checks the host
queue ready function.

Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/scsi/scsi.c |   19 ++++++++++++++-----
 1 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 36c92f9..5276e73 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -902,11 +902,20 @@ void scsi_adjust_queue_depth(struct scsi_device *sdev, int tagged, int tags)
 
 	spin_lock_irqsave(sdev->request_queue->queue_lock, flags);
 
-	/* Check to see if the queue is managed by the block layer.
-	 * If it is, and we fail to adjust the depth, exit. */
-	if (blk_queue_tagged(sdev->request_queue) &&
-	    blk_queue_resize_tags(sdev->request_queue, tags) != 0)
-		goto out;
+	/*
+	 * Check to see if the queue is managed by the block layer.
+	 * If it is, and we fail to adjust the depth, exit.
+	 *
+	 * Do not resize the tag map if it is a host wide share bqt,
+	 * because the size should be the hosts's can_queue. If there
+	 * is more IO than the LLD's can_queue (so there are not enuogh
+	 * tags) request_fn's host queue ready check will handle it.
+	 */
+	if (!sdev->host->bqt) {
+		if (blk_queue_tagged(sdev->request_queue) &&
+		    blk_queue_resize_tags(sdev->request_queue, tags) != 0)
+			goto out;
+	}
 
 	sdev->queue_depth = tags;
 	switch (tagged) {
-- 
1.5.4.1

[PATCH 2/3] qla4xxx: fix queue depth setting

[michaelc]

From: Mike Christie <michaelc@cs.wisc.edu>

We want to set the queue depth to something reasonable - not
the can_queue.

Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/scsi/qla4xxx/ql4_os.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index 5822dd5..88bebb1 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -46,6 +46,8 @@ MODULE_PARM_DESC(ql4xextended_error_logging,
 
 int ql4_mod_unload = 0;
 
+#define QL4_DEF_QDEPTH 32
+
 /*
  * SCSI host template entry points
  */
@@ -1387,7 +1389,7 @@ static int qla4xxx_slave_alloc(struct scsi_device *sdev)
 
 	sdev->hostdata = ddb;
 	sdev->tagged_supported = 1;
-	scsi_activate_tcq(sdev, sdev->host->can_queue);
+	scsi_activate_tcq(sdev, QL4_DEF_QDEPTH);
 	return 0;
 }
 
-- 
1.5.4.1

[PATCH 3/3] stex: fix queue depth setting

[michaelc]

From: Mike Christie <michaelc@cs.wisc.edu>

We want to set the queue depth to something reasonable - not
the can_queue.

Signed-off-by: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/scsi/stex.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/stex.c b/drivers/scsi/stex.c
index f308a03..3790906 100644
--- a/drivers/scsi/stex.c
+++ b/drivers/scsi/stex.c
@@ -467,7 +467,7 @@ stex_slave_alloc(struct scsi_device *sdev)
 	/* Cheat: usually extracted from Inquiry data */
 	sdev->tagged_supported = 1;
 
-	scsi_activate_tcq(sdev, sdev->host->can_queue);
+	scsi_activate_tcq(sdev, ST_CMD_PER_LUN);
 
 	return 0;
 }
-- 
1.5.4.1

Re: [PATCH 1/3] scsi: fix shared tag map setup

[Mike Christie]

michaelc@cs.wisc.edu wrote:
> From: Mike Christie <michaelc@cs.wisc.edu>
> 
> Currently qla4xxx and stex pass in their can_queue values into
> scsi_activate_tcq because they wanted the tag map that large.
> The problem with this is that it ends up also setting the queue
> depth to that large value. All we want to do this in this case
> is set the device queue depth and the other device settings.
> We do not need to touch the tag map sizing because the drivers
> had setup that map according to their can_queue limits when the
> shared map was created.
> 
> The scsi mid layer in request_fn will then handle the case where we
> have more requests than available tags when it checks the host
> queue ready function.
> 

Actually, I think this code needs one more fix to scsi_request_fn.

If we hit this in scsi_request_fn:

if (!(blk_queue_tagged(q) && !blk_queue_start_tag(q, req)))
	blkdev_dequeue_request(req);

blk_queue_tagged would be 1. blk_queue_start_tag could return 1 if we 
have too many commands. And then we will accidentally dequeue the 
request, and when we get to the host queue ready function, we could have 
raced with a completion which would have decreased the host_busy and so 
the command will be queued to the driver without a tag.

I will reroll these patches and add a fix to request_fn for this other 
problem and resend it all in one patchset.