scsi-mq: fix requests that use a separate CDB buffer

This patch fixes code such as the following with scsi-mq enabled:

     rq = blk_get_request(...);
     blk_rq_set_block_pc(rq);

     rq->cmd = my_cmd_buffer; /* separate CDB buffer */

     blk_execute_rq_nowait(...);

Code like this appears in e.g. sg_start_req() in drivers/scsi/sg.c (for large CDBs only). Without this patch, scsi_mq_prep_fn() will set rq->cmd back to rq->__cmd, causing the wrong CDB to be sent to the device.

16 byte cdb length capability is broken in

Quoted 9 lines · Tony Battersby

This patch fixes code such as the following with scsi-mq enabled:

     rq = blk_get_request(...);
     blk_rq_set_block_pc(rq);

     rq->cmd = my_cmd_buffer; /* separate CDB buffer */

     blk_execute_rq_nowait(...);

Quoted 3 lines

Code like this appears in e.g. sg_start_req() in drivers/scsi/sg.c (for large CDBs only). Without this patch, scsi_mq_prep_fn() will set rq->cmd back to rq->__cmd, causing the wrong CDB to be sent to the device.

Still looking at this one. 'sg_write_same --32' is my tool of choice. The target is scsi_debug which needs dif=2 (because mkp read the draft and only allows WRITE SAME(32) when the protection_level=2). Turned on command tracing and this is what I saw:

sd 7:0:0:0: scsi_debug: cmd 9e 10 00 00 00 00 00 00 00 00 00 00 00 20 00 00 sd 7:0:0:0: scsi_debug: cmd 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 08 24 6b d3 00 88 ff ff 20 00 c7 e8 00 00 00 00

So the WS(32) command did get through, it is the second command that arrived on its tail that is a worry. So yes, IMO it is broken.

Now the >16 byte cdb length in the sg driver
introduced in lk 3.17-rc1 was copied directly
from the bsg driver which has had that capability
for some time. Since your patch touches two files
in drivers/block I'm wondering the bsg driver's

Quoted 1 line

16 byte cdb length capability is broken in

lk 3.16 ?

Doug Gilbert

--
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

This patch fixes code such as the following with scsi-mq enabled:

     rq = blk_get_request(...);
     blk_rq_set_block_pc(rq);

     rq->cmd = my_cmd_buffer; /* separate CDB buffer */

     blk_execute_rq_nowait(...);

Code like this appears in e.g. sg_start_req() in drivers/scsi/sg.c (for large CDBs only). Without this patch, scsi_mq_prep_fn() will set rq->cmd back to rq->__cmd, causing the wrong CDB to be sent to the device.

Signed-off-by Tony Battersby <[email protected]>

For inclusion in 3.17 only.

diff -urpN linux-3.17.0-rc1-a/block/blk-core.c linux-3.17.0-rc1-b/block/blk-core.c

block/blk-core.c +0 -1

@@ -1252,7 +1252,6 @@ void blk_rq_set_block_pc(struct request
  	rq->__sector = (sector_t) -1;
  	rq->bio = rq->biotail = NULL;
  	memset(rq->__cmd, 0, sizeof(rq->__cmd));
-	rq->cmd = rq->__cmd;
  }
  EXPORT_SYMBOL(blk_rq_set_block_pc);
 

diff -urpN linux-3.17.0-rc1-a/block/blk-mq.c linux-3.17.0-rc1-b/block/blk-mq.c

block/blk-mq.c +2 -0

@@ -172,6 +172,8 @@ static void blk_mq_rq_ctx_init(struct re
  	/* tag was already set */
  	rq->errors = 0;
 
+	rq->cmd = rq->__cmd;
+
  	rq->extra_len = 0;
  	rq->sense_len = 0;
  	rq->resid_len = 0;

diff -urpN linux-3.17.0-rc1-a/drivers/scsi/scsi_lib.c linux-3.17.0-rc1-b/drivers/scsi/scsi_lib.c

drivers/scsi/scsi_lib.c +0 -2

@@ -1808,7 +1808,6 @@ static int scsi_mq_prep_fn(struct reques
 
  	cmd->tag = req->tag;
 
-	req->cmd = req->__cmd;
  	cmd->cmnd = req->cmd;
  	cmd->prot_op = SCSI_PROT_NORMAL;
 
 
--

Quoted 1 line

For inclusion in 3.17 only.

May want to check if blk-mq work in lk 3.16 and earlier breaks the bsg driver's capability to send SCSI cdbs that are longer than 16 bytes.