PATCH [3/5] qla2xxx: TCQ fixes

PATCH [3/5] qla2xxx: TCQ fixes

[Andrew Vasquez]

ChangeSet
  1.1867 04/07/12 09:38:28 andrew.vasquez@apc.qlogic.com +2 -0
  Correct usage of tag-command-queueing methods:
  
  o Properly call scsi_activate_tcq() rather than
    scsi_adjust_queue_depth().
  o Properly retrieve tag message from command via
    scsi_populate_tag_msg().

Signed-off-by: Andrew Vasquez <andrew.vasquez@qlogic.com>

 drivers/scsi/qla2xxx/qla_iocb.c |   16 +++++++++++-----
 drivers/scsi/qla2xxx/qla_os.c   |    2 +-
 2 files changed, 12 insertions(+), 6 deletions(-)

diff -Nru a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
--- a/drivers/scsi/qla2xxx/qla_iocb.c	2004-07-12 09:54:49 -07:00
+++ b/drivers/scsi/qla2xxx/qla_iocb.c	2004-07-12 09:54:49 -07:00
@@ -22,6 +22,8 @@
 #include <linux/blkdev.h>
 #include <linux/delay.h>
 
+#include <scsi/scsi_tcq.h>
+
 static inline uint16_t qla2x00_get_cmd_direction(struct scsi_cmnd *cmd);
 static inline cont_entry_t *qla2x00_prep_cont_type0_iocb(scsi_qla_host_t *);
 static inline cont_a64_entry_t *qla2x00_prep_cont_type1_iocb(scsi_qla_host_t *);
@@ -337,6 +339,7 @@
 	uint16_t	req_cnt;
 	uint16_t	tot_dsds;
 	device_reg_t	*reg;
+	char		tag[2];
 
 	/* Setup device pointers. */
 	ret = 0;
@@ -415,14 +418,17 @@
 	cmd_pkt->lun = cpu_to_le16(fclun->lun);
 
 	/* Update tagged queuing modifier */
-	cmd_pkt->control_flags = __constant_cpu_to_le16(CF_SIMPLE_TAG);
-	if (cmd->device->tagged_supported) {
-		switch (cmd->tag) {
-		case HEAD_OF_QUEUE_TAG:
+	if (scsi_populate_tag_msg(cmd, tag)) {
+		switch (tag[0]) {
+		case MSG_SIMPLE_TAG:
+			cmd_pkt->control_flags =
+			    __constant_cpu_to_le16(CF_SIMPLE_TAG);
+			break;
+		case MSG_HEAD_TAG:
 			cmd_pkt->control_flags =
 			    __constant_cpu_to_le16(CF_HEAD_TAG);
 			break;
-		case ORDERED_QUEUE_TAG:
+		case MSG_ORDERED_TAG:
 			cmd_pkt->control_flags =
 			    __constant_cpu_to_le16(CF_ORDERED_TAG);
 			break;
diff -Nru a/drivers/scsi/qla2xxx/qla_os.c b/drivers/scsi/qla2xxx/qla_os.c
--- a/drivers/scsi/qla2xxx/qla_os.c	2004-07-12 09:54:49 -07:00
+++ b/drivers/scsi/qla2xxx/qla_os.c	2004-07-12 09:54:49 -07:00
@@ -1784,7 +1784,7 @@
 
 		ql2xmaxqdepth = queue_depth;
 
-		scsi_adjust_queue_depth(sdev, MSG_ORDERED_TAG, queue_depth);
+		scsi_activate_tcq(sdev, queue_depth);
 
 		qla_printk(KERN_INFO, ha,
 		    "scsi(%d:%d:%d:%d): Enabled tagged queuing, queue "

Re: PATCH [3/5] qla2xxx: TCQ fixes

[James Bottomley]

On Mon, 2004-07-12 at 09:05, Andrew Vasquez wrote:
> ChangeSet
>   1.1867 04/07/12 09:38:28 andrew.vasquez@apc.qlogic.com +2 -0
>   Correct usage of tag-command-queueing methods:

I really don't think it is.

For the way the qla2xxx works, the correct tag usage would be to use per
HBA tags (which were introduced for aic7xxx but never used by them, so I
haven't actually created the mid-layer API), to populate your queue
handle field with the tag and to dispense with your outstanding_cmnds[]
array and use scsi_find_tag() to get back the command from the handle.

Since you'd be the first user of this API, I'd be happy to come up with
it for a willing victim^Wvolunteer.

James

Re: PATCH [3/5] qla2xxx: TCQ fixes

[Mike Christie]

James Bottomley wrote:
> On Mon, 2004-07-12 at 09:05, Andrew Vasquez wrote:
> 
>>ChangeSet
>>  1.1867 04/07/12 09:38:28 andrew.vasquez@apc.qlogic.com +2 -0
>>  Correct usage of tag-command-queueing methods:
> 
> 
> I really don't think it is.
> 
> For the way the qla2xxx works, the correct tag usage would be to use per
> HBA tags (which were introduced for aic7xxx but never used by them, so I
> haven't actually created the mid-layer API), to populate your queue
> handle field with the tag and to dispense with your outstanding_cmnds[]
> array and use scsi_find_tag() to get back the command from the handle.
> 
> Since you'd be the first user of this API, I'd be happy to come up with
> it for a willing victim^Wvolunteer.

Depending on the API, we might be able to use it for the linux-iscsi 
driver. Today the driver uses scsi_activate_tcq(), but does not uses the 
tag numbers. Instead it uses its own initiator task tags values for 
session wide identifiers, becuase we need tag values for iscsi ops. For 
example if we need to send a ping how should we get a tag from the mid 
layer? Or maybe becuase there are some transport specific tag values 
should we use someting else or keep it in the driver?

Mike

Re: PATCH [3/5] qla2xxx: TCQ fixes

[Brian King]

Mike Christie wrote:
> James Bottomley wrote:
> 
>> On Mon, 2004-07-12 at 09:05, Andrew Vasquez wrote:
>>
>>> ChangeSet
>>>  1.1867 04/07/12 09:38:28 andrew.vasquez@apc.qlogic.com +2 -0
>>>  Correct usage of tag-command-queueing methods:
>>
>>
>>
>> I really don't think it is.
>>
>> For the way the qla2xxx works, the correct tag usage would be to use per
>> HBA tags (which were introduced for aic7xxx but never used by them, so I
>> haven't actually created the mid-layer API), to populate your queue
>> handle field with the tag and to dispense with your outstanding_cmnds[]
>> array and use scsi_find_tag() to get back the command from the handle.
>>
>> Since you'd be the first user of this API, I'd be happy to come up with
>> it for a willing victim^Wvolunteer.
> 
> 
> Depending on the API, we might be able to use it for the linux-iscsi 
> driver. Today the driver uses scsi_activate_tcq(), but does not uses the 
> tag numbers. Instead it uses its own initiator task tags values for 
> session wide identifiers, becuase we need tag values for iscsi ops. For 
> example if we need to send a ping how should we get a tag from the mid 
> layer? Or maybe becuase there are some transport specific tag values 
> should we use someting else or keep it in the driver?

ipr is not interested in the tag numbers either, as the adapter 
microcode generates them.

-- 
Brian King
eServer Storage I/O
IBM Linux Technology Center

Re: PATCH [3/5] qla2xxx: TCQ fixes

[James Bottomley]

On Mon, 2004-07-12 at 19:19, Mike Christie wrote:
> Depending on the API, we might be able to use it for the linux-iscsi 
> driver. Today the driver uses scsi_activate_tcq(), but does not uses the 
> tag numbers. Instead it uses its own initiator task tags values for 
> session wide identifiers, becuase we need tag values for iscsi ops. For 
> example if we need to send a ping how should we get a tag from the mid 
> layer? Or maybe becuase there are some transport specific tag values 
> should we use someting else or keep it in the driver?

The only clean way I can see of doing it is to store the tags map in the
host structure.  That will pretty much require one host per transport
end point.  Is the iSCSI driver moving in that direction?

James

Re: PATCH [3/5] qla2xxx: TCQ fixes

[James Bottomley]

On Mon, 2004-07-12 at 21:28, Brian King wrote:
> ipr is not interested in the tag numbers either, as the adapter 
> microcode generates them.

Well, so does the qla2xxx.  However, all such implementations use driver
to controller handles which are direct tag equivalents (even if they're
not the actual tags sent over the wire). "Tag" in this sense merely
means identifying number from driver to host.

James

RE: PATCH [3/5] qla2xxx: TCQ fixes

[Andrew Vasquez]

On Monday, July 12, 2004 2:41 PM, James Bottomley wrote:
> 
> For the way the qla2xxx works, the correct tag usage would be to use
> per HBA tags (which were introduced for aic7xxx but never used by
> them, so I haven't actually created the mid-layer API), to populate
> your queue handle field with the tag and to dispense with your
> outstanding_cmnds[] array
>

Ok, I'll bite.  What did you have in mind? I've taken a quick peek
through the scsi code, the per-hba tag stuff must still be in your
head...

> and use scsi_find_tag() to get back the command from the handle. 
> 

Just a small comment, isn't the scsi_populate_tag_msg() function
quietly truncating the request->tag value?

Regards,
Andrew Vasquez

RE: PATCH [3/5] qla2xxx: TCQ fixes

[James Bottomley]

On Tue, 2004-07-13 at 12:24, Andrew Vasquez wrote:
> Ok, I'll bite.  What did you have in mind? I've taken a quick peek
> through the scsi code, the per-hba tag stuff must still be in your
> head...

Well, it exsits in the block layer (the ability to specify an existing
tag map to blk_queue_init_tags().  Jens is going to expose a bit more of
the tag map machinary to make that useful, then for host global tags,
we'll generate a single tag map and share it amongst all the queues.

> Just a small comment, isn't the scsi_populate_tag_msg() function
> quietly truncating the request->tag value?

Not for SPI, no.  However, we probably need a different API for the
sixteen bit tag values FC can use?

James