vmw_pvscsi: fixup tagging
Subsystems (2)
PATCH 1/1 vmw_pvscsi: fixup tagging
The request (and SCSI command) tag is the tag number assigned by the generic block-tagging code, not the SCSI-II tag messages. Those are represented by the device flags 'tagged_supported', 'simple_tags', and 'ordered_tags'.
(The SCSI midlayer doesn't use HEAD_OF_QUEUE tags).
So fixup vmw_pvscsi to assign the correct tag type.
--- drivers/scsi/vmw_pvscsi.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
drivers/scsi/vmw_pvscsi.c +2 -3
@@ -724,9 +724,8 @@ static int pvscsi_queue_ring(struct pvscsi_adapter *adapter, e->tag =SIMPLE_QUEUE_TAG ;if (sdev->tagged_supported && - (cmd->tag ==HEAD_OF_QUEUE_TAG || - cmd->tag ==ORDERED_QUEUE_TAG )) - e->tag = cmd->tag; + sdev->ordered_tags) + e->tag =ORDERED_QUEUE_TAG ;if (cmd->sc_data_direction ==DMA_FROM_DEVICE ) e->flags =PVSCSI_FLAG_CMD_DIR_TOHOST ;
Signature · 1 line
1.8.5.2
Can I get a review from VMWare for this one, please?
Quoted 35 lines · Hannes Reinecke
The request (and SCSI command) tag is the tag number assigned by the generic block-tagging code, not the SCSI-II tag messages. Those are represented by the device flags '
tagged_supported', 'simple_tags', and 'ordered_tags'.
(The SCSI midlayer doesn't useHEAD_OF_QUEUEtags).
So fixupvmw_pvscsito assign the correct tag type.Signed-off-by Hannes Reinecke <[email protected]>--- drivers/scsi/vmw_pvscsi.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)drivers/scsi/vmw_pvscsi.c +2
-3@@ -724,9 +724,8 @@ static int pvscsi_queue_ring(struct pvscsi_adapter *adapter, e->tag =SIMPLE_QUEUE_TAG ;if (sdev->tagged_supported && - (cmd->tag ==HEAD_OF_QUEUE_TAG || - cmd->tag ==ORDERED_QUEUE_TAG )) - e->tag = cmd->tag; + sdev->ordered_tags) + e->tag =ORDERED_QUEUE_TAG ;if (cmd->sc_data_direction ==DMA_FROM_DEVICE ) e->flags =PVSCSI_FLAG_CMD_DIR_TOHOST ;Signature · 6 lines
1.8.5.2 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
---end quoted text---
Hi Christoph,
Thanks for the change. Sorry for the delay. The change looks fine to me. I just have a question.
The comment in include/scsi/scsi_cmnd.h says:
struct scsi_cmnd {
...
unsigned char tag; /* SCSI-II queued command tag */
}
Is that comment not right? Should we update that too?
Thanks!
Arvind
________________________________________
From: Christoph Hellwig <[email protected]>
Sent: Friday, October 17, 2014 6:16 AM
To: Hannes Reinecke
Cc: Arvind Kumar; [email protected]; James Bottomley; [email protected]
Subject: Re: [PATCH] vmw_pvscsi: fixup tagging
Can I get a review from VMWare for this one, please?
Quoted 31 lines · Hannes Reinecke
The request (and SCSI command) tag is the tag number assigned by the generic block-tagging code, not the SCSI-II tag messages. Those are represented by the device flags '
tagged_supported', 'simple_tags', and 'ordered_tags'.
(The SCSI midlayer doesn't useHEAD_OF_QUEUEtags).
So fixupvmw_pvscsito assign the correct tag type.Signed-off-by Hannes Reinecke <[email protected]>--- drivers/scsi/vmw_pvscsi.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)drivers/scsi/vmw_pvscsi.c +2
-4@@ -724,9 +724,8 @@ static int pvscsi_queue_ring(struct pvscsi_adapter *adapter, e->tag =SIMPLE_QUEUE_TAG ;if (sdev->tagged_supported && - (cmd->tag ==HEAD_OF_QUEUE_TAG || - cmd->tag ==ORDERED_QUEUE_TAG )) - e->tag = cmd->tag; + sdev->ordered_tags) + e->tag =ORDERED_QUEUE_TAG ;if (cmd->sc_data_direction ==DMA_FROM_DEVICE ) e->flags =PVSCSI_FLAG_CMD_DIR_TOHOST ; --1.8.5.2
Quoted 4 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [email protected]
More majordomo info at https://urldefense.proofpoint.com/v1/url?u=http://vger.kernel.org/majordomo-info.html&k=oIvRg1%2BdGAgOoM1BIlLLqw%3D%3D%0A&r=5bh61bBshOhAkuAAXcD9BERI%2F4iK5qLrSSxbaPeaJh4%3D%0A&m=3T3CxXvbfPAK%2FDIE7aWB7MFmojVqfTOyoVs1VP7639M%3D%0A&s=97965a758f2187327667f0a727748779fd8dcdd81eb2da9d33f694fe7893e5d3
---end quoted text---
Quoted 4 lines · Arvind Kumar
Hi Christoph,
Thanks for the change. Sorry for the delay. The change looks fine to me. I just have a question.
Quoted 8 lines
The comment in
include/scsi/scsi_cmnd.hsays:struct
scsi_cmnd{
...
unsigned char tag; /* SCSI-II queued command tag */
}Is that comment not right? Should we update that too?
The comment looks very confusing and I'm fine with a patch that just removes the comment. That being said Hannes has patches to remove the whole field which I'd like to merge once he fixes up the remaining issues, so it might not worth the effort.
Quoted 4 lines · Arvind Kumar
Hi Christoph,
Thanks for the change. Sorry for the delay. The change looks fine to me. I just have a question.
Quoted 9 lines
The comment in
include/scsi/scsi_cmnd.hsays:struct
scsi_cmnd{
...
unsigned char tag; /* SCSI-II queued command tag */
}Is that comment not right? Should we update that too?
The 'tag' field from the scsi_cmnd is indeed meant for the SCSI-II queued command tag. But due to recent changes 'struct request' also contains a tag number which is used to implement a tag map.
So the 'tag; field from struct scsi_cmnd is basically obsolete,
and we're working on removing it.
Cheers,
Hannes
Signature · 8 lines
Dr. Hannes Reinecke zSeries & Storage [email protected] +49 911 74053 688 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Arvind Kumar
Acked-byThanks Hannes and Christoph for the answers. The patch to vmw_pvscsi.c looks fine to me.
Thanks!
Arvind
________________________________________
From: Hannes Reinecke <[email protected]>
Sent: Saturday, October 18, 2014 10:42 AM
To: Arvind Kumar; Christoph Hellwig
Cc: [email protected]; James Bottomley; [email protected]
Subject: Re: [PATCH] vmw_pvscsi: fixup tagging
Quoted 4 lines · Arvind Kumar
Hi Christoph,
Thanks for the change. Sorry for the delay. The change looks fine to me. I just have a question.
Quoted 9 lines
The comment in
include/scsi/scsi_cmnd.hsays:struct
scsi_cmnd{
...
unsigned char tag; /* SCSI-II queued command tag */
}Is that comment not right? Should we update that too?
The 'tag' field from the scsi_cmnd is indeed meant for the SCSI-II queued command tag. But due to recent changes 'struct request' also contains a tag number which is used to implement a tag map. So the 'tag; field from struct scsi_cmnd is basically obsolete, and we're working on removing it.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
[email protected] +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg) --
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Christoph Hellwig
Acked-by Arvind KumarFYI, I've updated the patch a bit in my tree to always set e->tag to SIMPLE_QUEUE_TAG. The rationale for that is that we actually never set MSG_ORDERED_TAG for other drivers since 2010, and fixing this up properly in this driver makes it conflict less with my patch series to fix up more mess in this area.
Updated patch below:
---
From: Hannes Reinecke <[email protected]>
Subject: vmw_pvscsi: fixup tagging
The request (and SCSI command) tag is the tag number assigned by the generic block-tagging code, not the SCSI-II tag messages. Those are represented by the device flags 'tagged_supported', 'simple_tags', and 'ordered_tags'.
(The SCSI midlayer doesn't use HEAD_OF_QUEUE tags).
So fixup vmw_pvscsi to assign the correct tag type.
--- drivers/scsi/vmw_pvscsi.c | 4 ---- 1 file changed, 4 deletions(-)
drivers/scsi/vmw_pvscsi.c +0 -4
@@ -723,10 +723,6 @@ static int pvscsi_queue_ring(struct pvscsi_adapter *adapter,memcpy (e->cdb, cmd->cmnd, e->cdbLen); e->tag =SIMPLE_QUEUE_TAG ; -if (sdev->tagged_supported && - (cmd->tag ==HEAD_OF_QUEUE_TAG || - cmd->tag ==ORDERED_QUEUE_TAG )) - e->tag = cmd->tag;if (cmd->sc_data_direction ==DMA_FROM_DEVICE ) e->flags =PVSCSI_FLAG_CMD_DIR_TOHOST ;
Signature · 1 line
1.9.1