[PATCH] vmw_pvscsi: fixup tagging

[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 use HEAD_OF_QUEUE tags).
So fixup vmw_pvscsi to assign the correct tag type.

Signed-off-by: Hannes Reinecke <***@suse.de>
---
drivers/scsi/vmw_pvscsi.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/vmw_pvscsi.c b/drivers/scsi/vmw_pvscsi.c
index 598f65e..d18df8c 100644
--- a/drivers/scsi/vmw_pvscsi.c
+++ b/drivers/scsi/vmw_pvscsi.c
@@ -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;

[Christoph Hellwig]

Can I get a review from VMWare for this one, please?

Post by 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 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(-)
diff --git a/drivers/scsi/vmw_pvscsi.c b/drivers/scsi/vmw_pvscsi.c
index 598f65e..d18df8c 100644
--- a/drivers/scsi/vmw_pvscsi.c
+++ b/drivers/scsi/vmw_pvscsi.c
@@ -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
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

---end quoted text---
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Arvind Kumar]

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 <***@infradead.org>
Sent: Friday, October 17, 2014 6:16 AM
To: Hannes Reinecke
Cc: Arvind Kumar; pv-***@vmware.com; James Bottomley; linux-***@vger.kernel.org
Subject: Re: [PATCH] vmw_pvscsi: fixup tagging

Can I get a review from VMWare for this one, please?

Post by 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 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(-)
diff --git a/drivers/scsi/vmw_pvscsi.c b/drivers/scsi/vmw_pvscsi.c
index 598f65e..d18df8c 100644
--- a/drivers/scsi/vmw_pvscsi.c
+++ b/drivers/scsi/vmw_pvscsi.c
@@ -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
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
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---
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Christoph Hellwig]

Post by Arvind Kumar
Hi Christoph,
Thanks for the change. Sorry for the delay. The change looks fine to me. I just have a question.
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.

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Hannes Reinecke]

Post by Arvind Kumar
Hi Christoph,
Thanks for the change. Sorry for the delay. The change looks fine to =

me. I just have a question.

Post by Arvind Kumar
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=20
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
--=20
Dr. Hannes Reinecke zSeries & Storage
***@suse.de [phone]
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg
GF: J. Hawn, J. Guild, F. Imend=F6rffer, HRB 16746 (AG N=FCrnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" i=
n
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Arvind Kumar]

Thanks Hannes and Christoph for the answers. The patch to vmw_pvscsi.c =
looks fine to me.

Acked-by: Arvind Kumar <***@vmware.com>

Thanks!
Arvind
________________________________________
=46rom: Hannes Reinecke <***@suse.de>
Sent: Saturday, October 18, 2014 10:42 AM
To: Arvind Kumar; Christoph Hellwig
Cc: pv-***@vmware.com; James Bottomley; linux-***@vger.kernel.org
Subject: Re: [PATCH] vmw_pvscsi: fixup tagging

Post by Arvind Kumar
Hi Christoph,
Thanks for the change. Sorry for the delay. The change looks fine to =

me. I just have a question.

Post by Arvind Kumar
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
***@suse.de [phone]
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg
GF: J. Hawn, J. Guild, F. Imend=F6rffer, HRB 16746 (AG N=FCrnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" i=
n
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html