p00002_scsi_tcq_qdepth_fix
PATCH 1/1 p00002_scsi_tcq_qdepth_fix
Signature · 4 lines
-- Brian King eServer Storage I/O IBM Linux Technology Center
Currently, it is possible to call scsi_activate_tcq with a small queue depth, then later call scsi_adjust_queue_depth with a larger queue depth. This results in the scsi layer having a larger queue depth than the block layer knows about. This results in these additional commands being issued as untagged ops rather than tagged ops. This patch changes scsi_activate_tcq to call blk_queue_init_tags with the maximum supported number of tags so this cannot occur.
---
linux-2.6.8-rc2-bjking1/drivers/scsi/scsi.c | 2 +- linux-2.6.8-rc2-bjking1/include/scsi/scsi_tcq.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
include/scsi/scsi_tcq.h renamed from include/scsi/scsi_tcq.h~scsi_tcq_qdepth_fix +2 -2
@@ -11,7 +11,7 @@#define MSG_ORDERED_TAG 0x22#define SCSI_NO_TAG (-1)/* identify no tag in use */ - +#define SCSI_MAX_TAGS 256/** * scsi_activate_tcq - turn on tag command queueing
@@ -30,7 +30,7 @@ static inline void scsi_activate_tcq(strif (sdev->tagged_supported) {spin_lock_irqsave (sdev->request_queue->queue_lock, flags);if (!blk_queue_tagged (sdev->request_queue)) -blk_queue_init_tags (sdev->request_queue, depth,NULL ); +blk_queue_init_tags (sdev->request_queue,SCSI_MAX_TAGS ,NULL );spin_unlock_irqrestore (sdev->request_queue->queue_lock, flags);scsi_adjust_queue_depth (sdev,MSG_ORDERED_TAG , depth); }
drivers/scsi/scsi.c renamed from drivers/scsi/scsi.c~scsi_tcq_qdepth_fix +1 -1
@@ -902,7 +902,7 @@ void scsi_adjust_queue_depth(struct scsi* we allocate a struct scsi_command for each of these and keep it * around forever. Too deep of a depth just wastes memory. */ -if (tags >256 ) +if (tags >SCSI_MAX_TAGS )return ;spin_lock_irqsave (&device_request_lock, flags);
_
Quoted 6 lines · Brian King
Currently, it is possible to call
scsi_activate_tcqwith a small queue depth, then later callscsi_adjust_queue_depthwith a larger queue depth. This results in the scsi layer having a larger queue depth than the block layer knows about. This results in these additional commands being issued as untagged ops rather than tagged ops. This patch changesscsi_activate_tcqto callblk_queue_init_tagswith the maximum supported number of tags so this cannot occur.
Sorry, been away at conferences with not enough time to remember what went on here.
The reason it looks the way it does is historical...when the blk layer tcq interfaces were created, there was no way to resize the queue. Jens later added resize (for me) and I forgot to incorporate it into the code.
Another small point is that the max number of tags can be greater than
256. 256 is a SPI limit only (and even the qla1280, a SPI card which could use the tag as its global queue index would take > 256). The limit in scsi_adjust_queue_depth has long since been obsoleted by our dynamic command allocation.
I think the attached should work correctly (as long as it compiles...I coded it up on the flight home).
James
===== drivers/scsi/scsi.c 1.145 vs edited =====
drivers/scsi/scsi.c +8 -7
@@ -897,15 +897,15 @@*/ if (tags <=0 )return ; -/* -* Limit max queue depth on a single lun to 256 for now. Remember, -* we allocate a struct scsi_command for each of these and keep it -* around forever. Too deep of a depth just wastes memory. -*/ -if (tags >256 ) -return ;spin_lock_irqsave (&device_request_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; + sdev->queue_depth = tags;switch (tagged) {case MSG_ORDERED_TAG :
@@ -926,6 +926,7 @@ sdev->queue_depth = tags;break ; } + out:spin_unlock_irqrestore (&device_request_lock, flags); }
James,
Ran into a couple small issues with the patch. First, blk_queue_resize_tags wasn't getting exported, so I sent Jens a patch to fix this, which he has now sent upstream. Second, the comment above blk_queue_resize_tags says the queue_lock must be held when calling this routine. Attached is an updated patch that grabs the lock. Other than that, it looks good.
Thanks,
Brian
Quoted 65 lines · James Bottomley
Quoted 6 lines · Brian King
Currently, it is possible to call
scsi_activate_tcqwith a small queue depth, then later callscsi_adjust_queue_depthwith a larger queue depth. This results in the scsi layer having a larger queue depth than the block layer knows about. This results in these additional commands being issued as untagged ops rather than tagged ops. This patch changesscsi_activate_tcqto callblk_queue_init_tagswith the maximum supported number of tags so this cannot occur.Sorry, been away at conferences with not enough time to remember what went on here.
The reason it looks the way it does is historical...when the blk layer tcq interfaces were created, there was no way to resize the queue. Jens later added resize (for me) and I forgot to incorporate it into the code.
Another small point is that the max number of tags can be greater than
256. 256 is a SPI limit only (and even the qla1280, a SPI card which could use the tag as its global queue index would take > 256). The limit inscsi_adjust_queue_depthhas long since been obsoleted by our dynamic command allocation.I think the attached should work correctly (as long as it compiles...I coded it up on the flight home).
James
=====
drivers/scsi/scsi.c1.145 vs edited =====drivers/scsi/scsi.c +8
-7@@ -897,15 +897,15 @@*/ if (tags <=0 )return ; -/* -* Limit max queue depth on a single lun to 256 for now. Remember, -* we allocate a struct scsi_command for each of these and keep it -* around forever. Too deep of a depth just wastes memory. -*/ -if (tags >256 ) -return ;spin_lock_irqsave (&device_request_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; + sdev->queue_depth = tags;switch (tagged) {case MSG_ORDERED_TAG :@@ -926,6 +926,7 @@ sdev->queue_depth = tags;break ; } + out:spin_unlock_irqrestore (&device_request_lock, flags); }
Signature · 3 lines
Brian King eServer Storage I/O IBM Linux Technology Center
From: James Bottomley <[email protected]>
Quoted 6 lines · Brian King
Currently, it is possible to call
scsi_activate_tcqwith a small queue depth, then later callscsi_adjust_queue_depthwith a larger queue depth. This results in the scsi layer having a larger queue depth than the block layer knows about. This results in these additional commands being issued as untagged ops rather than tagged ops. This patch changesscsi_activate_tcqto callblk_queue_init_tagswith the maximum supported number of tags so this cannot occur.
Sorry, been away at conferences with not enough time to remember what went on here.
The reason it looks the way it does is historical...when the blk layer tcq interfaces were created, there was no way to resize the queue. Jens later added resize (for me) and I forgot to incorporate it into the code.
Another small point is that the max number of tags can be greater than
256. 256 is a SPI limit only (and even the qla1280, a SPI card which could use the tag as its global queue index would take > 256). The limit in scsi_adjust_queue_depth has long since been obsoleted by our dynamic command allocation.
I think the attached should work correctly (as long as it compiles...I coded it up on the flight home).
James
===== drivers/scsi/scsi.c 1.145 vs edited =====
---
linux-2.6.8-rc3-bjking1/drivers/scsi/scsi.c | 17 ++++++++++------- 1 files changed, 10 insertions(+), 7 deletions(-)
drivers/scsi/scsi.c renamed from drivers/scsi/scsi.c~scsi_tcq_resize +10 -7
@@ -897,15 +897,16 @@ void scsi_adjust_queue_depth(struct scsi*/ if (tags <=0 )return ; -/* -* Limit max queue depth on a single lun to 256 for now. Remember, -* we allocate a struct scsi_command for each of these and keep it -* around forever. Too deep of a depth just wastes memory. -*/ -if (tags >256 ) -return ;spin_lock_irqsave (&device_request_lock, flags); +spin_lock (sdev->request_queue->queue_lock); + +/* 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; + sdev->queue_depth = tags;switch (tagged) {case MSG_ORDERED_TAG :
@@ -926,6 +927,8 @@ void scsi_adjust_queue_depth(struct scsi sdev->queue_depth = tags;break ; } + out: +spin_unlock (sdev->request_queue->queue_lock);spin_unlock_irqrestore (&device_request_lock, flags); }
_