remove struct scsi_request

remove struct scsi_request

[Christoph Hellwig]

Hi Martin,

with the recent removal of the REQ_OP_SCSI_{IN,OUT} based passthrough
from non-scsi drivers, the need for the scsi_request structure went
away as well.  As all submitters of SCSI passthrough requests are using
the SCSI midlayer now they can just fill out the scsi_cmnd directly.

Diffstat:
 b/drivers/ata/libata-scsi.c           |    4 -
 b/drivers/block/pktcdvd.c             |    8 +-
 b/drivers/cdrom/cdrom.c               |    1 
 b/drivers/scsi/scsi_bsg.c             |   42 +++++-------
 b/drivers/scsi/scsi_debugfs.c         |    6 -
 b/drivers/scsi/scsi_error.c           |   38 +++++-----
 b/drivers/scsi/scsi_ioctl.c           |   75 +++++++++------------
 b/drivers/scsi/scsi_lib.c             |  119 ++++++++++++++--------------------
 b/drivers/scsi/scsi_logging.c         |    5 -
 b/drivers/scsi/scsi_transport_sas.c   |    1 
 b/drivers/scsi/sd.c                   |   28 --------
 b/drivers/scsi/sg.c                   |   44 ++++--------
 b/drivers/scsi/sr.c                   |   30 ++++----
 b/drivers/scsi/st.c                   |   30 ++++----
 b/drivers/scsi/ufs/ufshpb.c           |   22 ++----
 b/drivers/target/target_core_pscsi.c  |   67 +++++++------------
 b/drivers/target/target_core_pscsi.h  |    4 -
 b/drivers/usb/storage/cypress_atacb.c |    1 
 b/drivers/usb/storage/isd200.c        |    4 -
 b/include/linux/bsg-lib.h             |    1 
 b/include/scsi/scsi_cmnd.h            |   16 +---
 b/include/scsi/scsi_eh.h              |    4 -
 include/scsi/scsi_request.h           |   31 --------
 23 files changed, 225 insertions(+), 356 deletions(-)

[PATCH 1/8] bsg: don't include scsi_request.h in bsg-lib.h

[Christoph Hellwig]

There is no need for any of the information from scsi_request.h in
bsg-lib.h.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/linux/bsg-lib.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/linux/bsg-lib.h b/include/linux/bsg-lib.h
index 6b211323a489c..9e97ced2896df 100644
--- a/include/linux/bsg-lib.h
+++ b/include/linux/bsg-lib.h
@@ -10,7 +10,6 @@
 #define _BLK_BSG_
 
 #include <linux/blkdev.h>
-#include <scsi/scsi_request.h>
 
 struct bsg_job;
 struct request;
-- 
2.30.2

Re: [PATCH 1/8] bsg: don't include scsi_request.h in bsg-lib.h

[Bart Van Assche]

On 2/22/22 06:04, Christoph Hellwig wrote:
> There is no need for any of the information from scsi_request.h in
> bsg-lib.h.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

[PATCH 2/8] target/pscsi: remove struct pscsi_plugin_task

[Christoph Hellwig]

Copy directly from the se_cmd CDB to the one in the scsi_request.  This
temporarily limits the pscsi backend to supporting only up to 16 byte
CDBs, but this restriction will be lifted later in this series.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/target/target_core_pscsi.c | 51 ++++++++++--------------------
 drivers/target/target_core_pscsi.h |  4 ---
 2 files changed, 17 insertions(+), 38 deletions(-)

diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c
index 807d06ecadee2..622158ed5d9ce 100644
--- a/drivers/target/target_core_pscsi.c
+++ b/drivers/target/target_core_pscsi.c
@@ -593,16 +593,14 @@ static void pscsi_complete_cmd(struct se_cmd *cmd, u8 scsi_status,
 {
 	struct pscsi_dev_virt *pdv = PSCSI_DEV(cmd->se_dev);
 	struct scsi_device *sd = pdv->pdv_sd;
-	struct pscsi_plugin_task *pt = cmd->priv;
-	unsigned char *cdb;
+	unsigned char *cdb = cmd->priv;
+
 	/*
-	 * Special case for REPORT_LUNs handling where pscsi_plugin_task has
-	 * not been allocated because TCM is handling the emulation directly.
+	 * Special case for REPORT_LUNs which is emulated and not passed on.
 	 */
-	if (!pt)
+	if (!cdb)
 		return;
 
-	cdb = &pt->pscsi_cdb[0];
 	/*
 	 * Hack to make sure that Write-Protect modepage is set if R/O mode is
 	 * forced.
@@ -963,30 +961,14 @@ pscsi_execute_cmd(struct se_cmd *cmd)
 	struct scatterlist *sgl = cmd->t_data_sg;
 	u32 sgl_nents = cmd->t_data_nents;
 	struct pscsi_dev_virt *pdv = PSCSI_DEV(cmd->se_dev);
-	struct pscsi_plugin_task *pt;
 	struct request *req;
 	sense_reason_t ret;
 
-	/*
-	 * Dynamically alloc cdb space, since it may be larger than
-	 * TCM_MAX_COMMAND_SIZE
-	 */
-	pt = kzalloc(sizeof(*pt) + scsi_command_size(cmd->t_task_cdb), GFP_KERNEL);
-	if (!pt) {
-		return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
-	}
-	cmd->priv = pt;
-
-	memcpy(pt->pscsi_cdb, cmd->t_task_cdb,
-		scsi_command_size(cmd->t_task_cdb));
-
 	req = scsi_alloc_request(pdv->pdv_sd->request_queue,
 			cmd->data_direction == DMA_TO_DEVICE ?
 			REQ_OP_DRV_OUT : REQ_OP_DRV_IN, 0);
-	if (IS_ERR(req)) {
-		ret = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
-		goto fail;
-	}
+	if (IS_ERR(req))
+		return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
 
 	if (sgl) {
 		ret = pscsi_map_sg(cmd, sgl, sgl_nents, req);
@@ -996,8 +978,12 @@ pscsi_execute_cmd(struct se_cmd *cmd)
 
 	req->end_io = pscsi_req_done;
 	req->end_io_data = cmd;
-	scsi_req(req)->cmd_len = scsi_command_size(pt->pscsi_cdb);
-	scsi_req(req)->cmd = &pt->pscsi_cdb[0];
+	scsi_req(req)->cmd_len = scsi_command_size(cmd->t_task_cdb);
+	if (scsi_req(req)->cmd_len > BLK_MAX_CDB) {
+		ret = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
+		goto fail_put_request;
+	}
+	memcpy(scsi_req(req)->cmd, cmd->t_task_cdb, scsi_req(req)->cmd_len);
 	if (pdv->pdv_sd->type == TYPE_DISK ||
 	    pdv->pdv_sd->type == TYPE_ZBC)
 		req->timeout = PS_TIMEOUT_DISK;
@@ -1005,6 +991,8 @@ pscsi_execute_cmd(struct se_cmd *cmd)
 		req->timeout = PS_TIMEOUT_OTHER;
 	scsi_req(req)->retries = PS_RETRY;
 
+	cmd->priv = scsi_req(req)->cmd;
+
 	blk_execute_rq_nowait(req, cmd->sam_task_attr == TCM_HEAD_TAG,
 			pscsi_req_done);
 
@@ -1012,8 +1000,6 @@ pscsi_execute_cmd(struct se_cmd *cmd)
 
 fail_put_request:
 	blk_mq_free_request(req);
-fail:
-	kfree(pt);
 	return ret;
 }
 
@@ -1041,14 +1027,13 @@ static sector_t pscsi_get_blocks(struct se_device *dev)
 static void pscsi_req_done(struct request *req, blk_status_t status)
 {
 	struct se_cmd *cmd = req->end_io_data;
-	struct pscsi_plugin_task *pt = cmd->priv;
 	int result = scsi_req(req)->result;
 	enum sam_status scsi_status = result & 0xff;
+	u8 *cdb = cmd->priv;
 
 	if (scsi_status != SAM_STAT_GOOD) {
 		pr_debug("PSCSI Status Byte exception at cmd: %p CDB:"
-			" 0x%02x Result: 0x%08x\n", cmd, pt->pscsi_cdb[0],
-			result);
+			" 0x%02x Result: 0x%08x\n", cmd, cdb[0], result);
 	}
 
 	pscsi_complete_cmd(cmd, scsi_status, scsi_req(req)->sense);
@@ -1060,14 +1045,12 @@ static void pscsi_req_done(struct request *req, blk_status_t status)
 		break;
 	default:
 		pr_debug("PSCSI Host Byte exception at cmd: %p CDB:"
-			" 0x%02x Result: 0x%08x\n", cmd, pt->pscsi_cdb[0],
-			result);
+			" 0x%02x Result: 0x%08x\n", cmd, cdb[0], result);
 		target_complete_cmd(cmd, SAM_STAT_CHECK_CONDITION);
 		break;
 	}
 
 	blk_mq_free_request(req);
-	kfree(pt);
 }
 
 static const struct target_backend_ops pscsi_ops = {
diff --git a/drivers/target/target_core_pscsi.h b/drivers/target/target_core_pscsi.h
index e8458b5e85c92..23d9a6e340d41 100644
--- a/drivers/target/target_core_pscsi.h
+++ b/drivers/target/target_core_pscsi.h
@@ -23,10 +23,6 @@ struct block_device;
 struct scsi_device;
 struct Scsi_Host;
 
-struct pscsi_plugin_task {
-	unsigned char pscsi_cdb[0];
-} ____cacheline_aligned;
-
 #define PDF_HAS_CHANNEL_ID	0x01
 #define PDF_HAS_TARGET_ID	0x02
 #define PDF_HAS_LUN_ID		0x04
-- 
2.30.2

[PATCH 3/8] scsi: don't memset the entire scsi_cmnd in scsi_init_command

[Christoph Hellwig]

Replace the big fat memset that requires saving and restoring various
fields with just initializing those fields that need initialization.

All the clearing to 0 ismoved to scsi_prepare_cmd as scsi_ioctl_reset
alreadly uses kzalloc to allocate a pre-zeroed command.

This is still conservative and can probably be optimizated further.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/scsi_lib.c | 61 ++++++++++++++++++++---------------------
 1 file changed, 29 insertions(+), 32 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index a1c18ba5e8d38..960795d469d8c 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1163,45 +1163,17 @@ static void scsi_cleanup_rq(struct request *rq)
 /* Called before a request is prepared. See also scsi_mq_prep_fn(). */
 void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd)
 {
-	void *buf = cmd->sense_buffer;
-	void *prot = cmd->prot_sdb;
 	struct request *rq = scsi_cmd_to_rq(cmd);
-	unsigned int flags = cmd->flags & SCMD_PRESERVED_FLAGS;
-	unsigned long jiffies_at_alloc;
-	int retries, to_clear;
-	bool in_flight;
-	int budget_token = cmd->budget_token;
-
-	if (!blk_rq_is_passthrough(rq) && !(flags & SCMD_INITIALIZED)) {
-		flags |= SCMD_INITIALIZED;
+
+	if (!blk_rq_is_passthrough(rq) && !(cmd->flags & SCMD_INITIALIZED)) {
+		cmd->flags |= SCMD_INITIALIZED;
 		scsi_initialize_rq(rq);
 	}
 
-	jiffies_at_alloc = cmd->jiffies_at_alloc;
-	retries = cmd->retries;
-	in_flight = test_bit(SCMD_STATE_INFLIGHT, &cmd->state);
-	/*
-	 * Zero out the cmd, except for the embedded scsi_request. Only clear
-	 * the driver-private command data if the LLD does not supply a
-	 * function to initialize that data.
-	 */
-	to_clear = sizeof(*cmd) - sizeof(cmd->req);
-	if (!dev->host->hostt->init_cmd_priv)
-		to_clear += dev->host->hostt->cmd_size;
-	memset((char *)cmd + sizeof(cmd->req), 0, to_clear);
-
 	cmd->device = dev;
-	cmd->sense_buffer = buf;
-	cmd->prot_sdb = prot;
-	cmd->flags = flags;
+	cmd->flags &= SCMD_PRESERVED_FLAGS;
 	INIT_LIST_HEAD(&cmd->eh_entry);
 	INIT_DELAYED_WORK(&cmd->abort_work, scmd_eh_abort_handler);
-	cmd->jiffies_at_alloc = jiffies_at_alloc;
-	cmd->retries = retries;
-	if (in_flight)
-		__set_bit(SCMD_STATE_INFLIGHT, &cmd->state);
-	cmd->budget_token = budget_token;
-
 }
 
 static blk_status_t scsi_setup_scsi_cmnd(struct scsi_device *sdev,
@@ -1586,10 +1558,35 @@ static blk_status_t scsi_prepare_cmd(struct request *req)
 	struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(req);
 	struct scsi_device *sdev = req->q->queuedata;
 	struct Scsi_Host *shost = sdev->host;
+	bool in_flight = test_bit(SCMD_STATE_INFLIGHT, &cmd->state);
 	struct scatterlist *sg;
 
 	scsi_init_command(sdev, cmd);
 
+	cmd->eh_eflags = 0;
+	cmd->allowed = 0;
+	cmd->prot_type = 0;
+	cmd->prot_flags = 0;
+	cmd->submitter = 0;
+	cmd->cmd_len = 0;
+	cmd->cmnd = NULL;
+	memset(&cmd->sdb, 0, sizeof(cmd->sdb));
+	cmd->underflow = 0;
+	cmd->transfersize = 0;
+	cmd->host_scribble = NULL;
+	cmd->result = 0;
+	cmd->extra_len = 0;
+	cmd->state = 0;
+	if (in_flight)
+		__set_bit(SCMD_STATE_INFLIGHT, &cmd->state);
+
+	/*
+	 * Only clear the driver-private command data if the LLD does not supply
+	 * a function to initialize that data.
+	 */
+	if (!shost->hostt->init_cmd_priv)
+		memset(cmd + 1, 0, shost->hostt->cmd_size);
+
 	cmd->prot_op = SCSI_PROT_NORMAL;
 	if (blk_rq_bytes(req))
 		cmd->sc_data_direction = rq_dma_dir(req);
-- 
2.30.2

Re: [PATCH 3/8] scsi: don't memset the entire scsi_cmnd in scsi_init_command

[John Garry]

On 22/02/2022 14:04, Christoph Hellwig wrote:

Hi Christoph,

>   static blk_status_t scsi_setup_scsi_cmnd(struct scsi_device *sdev,
> @@ -1586,10 +1558,35 @@ static blk_status_t scsi_prepare_cmd(struct request *req)
>   	struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(req);
>   	struct scsi_device *sdev = req->q->queuedata;
>   	struct Scsi_Host *shost = sdev->host;
> +	bool in_flight = test_bit(SCMD_STATE_INFLIGHT, &cmd->state);
>   	struct scatterlist *sg;
>   
>   	scsi_init_command(sdev, cmd);
>   
> +	cmd->eh_eflags = 0;
> +	cmd->allowed = 0;
> +	cmd->prot_type = 0;
> +	cmd->prot_flags = 0;
> +	cmd->submitter = 0;
> +	cmd->cmd_len = 0;
> +	cmd->cmnd = NULL;
> +	memset(&cmd->sdb, 0, sizeof(cmd->sdb));
> +	cmd->underflow = 0;
> +	cmd->transfersize = 0;
> +	cmd->host_scribble = NULL;
> +	cmd->result = 0;
> +	cmd->extra_len = 0;
> +	cmd->state = 0;

I am just wondering did you consider using struct_group() for safety?

I don't think the scsi_cmnd members have special ordering apart from 
co-locating related members.

Here's how it could look (on top of yours):

---8<----

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 0b5c4606f641..51d2afa2d4c9 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1549,18 +1549,8 @@ static blk_status_t scsi_prepare_cmd(struct 
request *req)

  	scsi_init_command(sdev, cmd);

-	cmd->eh_eflags = 0;
-	cmd->allowed = 0;
-	cmd->prot_type = 0;
-	cmd->prot_flags = 0;
-	cmd->submitter = 0;
-	memset(&cmd->sdb, 0, sizeof(cmd->sdb));
-	cmd->underflow = 0;
-	cmd->transfersize = 0;
-	cmd->host_scribble = NULL;
-	cmd->result = 0;
-	cmd->extra_len = 0;
-	cmd->state = 0;
+	memset((char *)cmd + offsetof(struct scsi_cmnd, to_be_zeroed), 0, 
sizeof(cmd->to_be_zeroed));
+
  	if (in_flight)
  		__set_bit(SCMD_STATE_INFLIGHT, &cmd->state);

diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 76c5eaeeb3b5..8d7a7186a3ab 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -73,10 +73,46 @@ struct scsi_cmnd {

  	struct rcu_head rcu;

-	int eh_eflags;		/* Used by error handlr */
-
  	int budget_token;

+	struct_group(to_be_zeroed,
+		int eh_eflags;		/* Used by error handlr */
+		int allowed;
+		unsigned char prot_type;
+		unsigned char prot_flags;
+		enum scsi_cmnd_submitter submitter;

...

---->8---

Thanks,
John

Re: [PATCH 3/8] scsi: don't memset the entire scsi_cmnd in scsi_init_command

[Christoph Hellwig]

On Wed, Feb 23, 2022 at 12:21:10PM +0000, John Garry wrote:
> I am just wondering did you consider using struct_group() for safety?
>
> I don't think the scsi_cmnd members have special ordering apart from 
> co-locating related members.
>
> Here's how it could look (on top of yours):

Besides being ugly as hell I don't see the benefit.  Quite contrary,
a lot of these fields are properly initialized later and we can drop
the zeroing as well, but I'd rather do that separately and with a proper
audit.

Re: [PATCH 3/8] scsi: don't memset the entire scsi_cmnd in scsi_init_command

[John Garry]

On 23/02/2022 12:39, Christoph Hellwig wrote:
> On Wed, Feb 23, 2022 at 12:21:10PM +0000, John Garry wrote:
>> I am just wondering did you consider using struct_group() for safety?
>>
>> I don't think the scsi_cmnd members have special ordering apart from
>> co-locating related members.
>>
>> Here's how it could look (on top of yours):
> 
> Besides being ugly as hell I don't see the benefit.  Quite contrary,
> a lot of these fields are properly initialized later and we can drop
> the zeroing as well, but I'd rather do that separately and with a proper
> audit.
> 
> .

ok, fine - so the init may be removed later.

For the record, advantages now could include:
- prob more efficient
- somewhat safer coding practice
- more condense C code

But, for sure, it's not so pretty to have this sub-struct.

Anyway, I don't feel too strongly about it and it was just a suggestion.

Thanks,
John

Re: [PATCH 3/8] scsi: don't memset the entire scsi_cmnd in scsi_init_command

[Christoph Hellwig]

On Wed, Feb 23, 2022 at 12:56:32PM +0000, John Garry wrote:
> For the record, advantages now could include:
> - prob more efficient
> - somewhat safer coding practice
> - more condense C code
>
> But, for sure, it's not so pretty to have this sub-struct.
>
> Anyway, I don't feel too strongly about it and it was just a suggestion.

In general most of the zeroing here should go away entirely.  Right
now that code is a bit convolute due to the fake EH scsi command that
also uses scsi_init_command.  Once that is sorted out (I've just pinged
Hannes for his series) scsi_init_command can be folded into
scsi_prepare_cmd and a lot more of this can be cleaned up.

Re: [PATCH 3/8] scsi: don't memset the entire scsi_cmnd in scsi_init_command

[Bart Van Assche]

On 2/23/22 04:58, Christoph Hellwig wrote:
> In general most of the zeroing here should go away entirely.  Right
> now that code is a bit convolute due to the fake EH scsi command that
> also uses scsi_init_command.  Once that is sorted out (I've just pinged
> Hannes for his series) scsi_init_command can be folded into
> scsi_prepare_cmd and a lot more of this can be cleaned up.

Several SCSI LLDs rely on the SCSI core zeroing the driver-private data.
I'd be more than happy if the code for zeroing of driver-private data 
would be pushed into the LLD drivers. This may require the introduction 
of a new per-command flag since that zeroing should only happen if 
scsi_queue_rq() decided to call scsi_prepare_cmd().

Bart.

Re: [PATCH 3/8] scsi: don't memset the entire scsi_cmnd in scsi_init_command

[Christoph Hellwig]

On Wed, Feb 23, 2022 at 12:16:24PM -0800, Bart Van Assche wrote:
> On 2/23/22 04:58, Christoph Hellwig wrote:
>> In general most of the zeroing here should go away entirely.  Right
>> now that code is a bit convolute due to the fake EH scsi command that
>> also uses scsi_init_command.  Once that is sorted out (I've just pinged
>> Hannes for his series) scsi_init_command can be folded into
>> scsi_prepare_cmd and a lot more of this can be cleaned up.
>
> Several SCSI LLDs rely on the SCSI core zeroing the driver-private data.
> I'd be more than happy if the code for zeroing of driver-private data would 
> be pushed into the LLD drivers. This may require the introduction of a new 
> per-command flag since that zeroing should only happen if scsi_queue_rq() 
> decided to call scsi_prepare_cmd().

If the driver provides a init_cmd_priv method it is not zeroed.  So if
a driver does not want it zeroed, it can always provide this callback,
including an empty one.

Re: [PATCH 3/8] scsi: don't memset the entire scsi_cmnd in scsi_init_command

[Bart Van Assche]

On 2/22/22 06:04, Christoph Hellwig wrote:
> Replace the big fat memset that requires saving and restoring various
> fields with just initializing those fields that need initialization.
> 
> All the clearing to 0 ismoved to scsi_prepare_cmd as scsi_ioctl_reset
                           ^
                    missing space?
> alreadly uses kzalloc to allocate a pre-zeroed command.
> 
> This is still conservative and can probably be optimizated further.
                                                  ^^^^^^^^^^^
                                                  optimized?
Anyway:

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

Re: [PATCH 3/8] scsi: don't memset the entire scsi_cmnd in scsi_init_command

[John Garry]

On 22/02/2022 14:04, Christoph Hellwig wrote:
> Replace the big fat memset that requires saving and restoring various
> fields with just initializing those fields that need initialization.
> 
> All the clearing to 0 ismoved to scsi_prepare_cmd as scsi_ioctl_reset

nit: "is moved"?

> alreadly uses kzalloc to allocate a pre-zeroed command.
> 
> This is still conservative and can probably be optimizated further.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

FWIW:

Reviewed-by: John Garry <john.garry@huawei.com>

But just a comment below.

> ---
>   drivers/scsi/scsi_lib.c | 61 ++++++++++++++++++++---------------------
>   1 file changed, 29 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index a1c18ba5e8d38..960795d469d8c 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -1163,45 +1163,17 @@ static void scsi_cleanup_rq(struct request *rq)
>   /* Called before a request is prepared. See also scsi_mq_prep_fn(). */
>   void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd)
>   {
> -	void *buf = cmd->sense_buffer;
> -	void *prot = cmd->prot_sdb;
>   	struct request *rq = scsi_cmd_to_rq(cmd);
> -	unsigned int flags = cmd->flags & SCMD_PRESERVED_FLAGS;
> -	unsigned long jiffies_at_alloc;
> -	int retries, to_clear;
> -	bool in_flight;
> -	int budget_token = cmd->budget_token;
> -
> -	if (!blk_rq_is_passthrough(rq) && !(flags & SCMD_INITIALIZED)) {
> -		flags |= SCMD_INITIALIZED;
> +
> +	if (!blk_rq_is_passthrough(rq) && !(cmd->flags & SCMD_INITIALIZED)) {
> +		cmd->flags |= SCMD_INITIALIZED;

Maybe I'm being dozy, but isn't this being cleared below *

>   		scsi_initialize_rq(rq);
>   	}
>   
> -	jiffies_at_alloc = cmd->jiffies_at_alloc;
> -	retries = cmd->retries;
> -	in_flight = test_bit(SCMD_STATE_INFLIGHT, &cmd->state);
> -	/*
> -	 * Zero out the cmd, except for the embedded scsi_request. Only clear
> -	 * the driver-private command data if the LLD does not supply a
> -	 * function to initialize that data.
> -	 */
> -	to_clear = sizeof(*cmd) - sizeof(cmd->req);
> -	if (!dev->host->hostt->init_cmd_priv)
> -		to_clear += dev->host->hostt->cmd_size;
> -	memset((char *)cmd + sizeof(cmd->req), 0, to_clear);
> -
>   	cmd->device = dev;
> -	cmd->sense_buffer = buf;
> -	cmd->prot_sdb = prot;
> -	cmd->flags = flags;
> +	cmd->flags &= SCMD_PRESERVED_FLAGS;

*

>   	INIT_LIST_HEAD(&cmd->eh_entry);
>   	INIT_DELAYED_WORK(&cmd->abort_work, scmd_eh_abort_handler);
> -	cmd->jiffies_at_alloc = jiffies_at_alloc;
> -	cmd->retries = retries;
> -	if (in_flight)
> -		__set_bit(SCMD_STATE_INFLIGHT, &cmd->state);
> -	cmd->budget_token = budget_token;
> -
>   }
>   
>   static blk_status_t scsi_setup_scsi_cmnd(struct scsi_device *sdev,
> @@ -1586,10 +1558,35 @@ static blk_status_t scsi_prepare_cmd(struct request *req)
>   	struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(req);
>   	struct scsi_device *sdev = req->q->queuedata;
>   	struct Scsi_Host *shost = sdev->host;
> +	bool in_flight = test_bit(SCMD_STATE_INFLIGHT, &cmd->state);
>   	struct scatterlist *sg;
>   
>   	scsi_init_command(sdev, cmd);
>   
> +	cmd->eh_eflags = 0;
> +	cmd->allowed = 0;
> +	cmd->prot_type = 0;
> +	cmd->prot_flags = 0;
> +	cmd->submitter = 0;
> +	cmd->cmd_len = 0;
> +	cmd->cmnd = NULL;
> +	memset(&cmd->sdb, 0, sizeof(cmd->sdb));
> +	cmd->underflow = 0;
> +	cmd->transfersize = 0;
> +	cmd->host_scribble = NULL;
> +	cmd->result = 0;
> +	cmd->extra_len = 0;
> +	cmd->state = 0;
> +	if (in_flight)
> +		__set_bit(SCMD_STATE_INFLIGHT, &cmd->state);
> +
> +	/*
> +	 * Only clear the driver-private command data if the LLD does not supply
> +	 * a function to initialize that data.
> +	 */
> +	if (!shost->hostt->init_cmd_priv)
> +		memset(cmd + 1, 0, shost->hostt->cmd_size);
> +
>   	cmd->prot_op = SCSI_PROT_NORMAL;
>   	if (blk_rq_bytes(req))
>   		cmd->sc_data_direction = rq_dma_dir(req);

Re: [PATCH 3/8] scsi: don't memset the entire scsi_cmnd in scsi_init_command

[Christoph Hellwig]

On Thu, Feb 24, 2022 at 08:28:48AM +0000, John Garry wrote:
>> +	if (!blk_rq_is_passthrough(rq) && !(cmd->flags & SCMD_INITIALIZED)) {
>> +		cmd->flags |= SCMD_INITIALIZED;
>
> Maybe I'm being dozy, but isn't this being cleared below *

with below you mean the

	cmd->flags &= SCMD_PRESERVED_FLAGS;

?  No, that doen't clear the flag, but all the others.

Re: [PATCH 3/8] scsi: don't memset the entire scsi_cmnd in scsi_init_command

[John Garry]

On 24/02/2022 16:27, Christoph Hellwig wrote:
> On Thu, Feb 24, 2022 at 08:28:48AM +0000, John Garry wrote:
>>> +	if (!blk_rq_is_passthrough(rq) && !(cmd->flags & SCMD_INITIALIZED)) {
>>> +		cmd->flags |= SCMD_INITIALIZED;
>> Maybe I'm being dozy, but isn't this being cleared below *
> with below you mean the
> 
> 	cmd->flags &= SCMD_PRESERVED_FLAGS;
> 
> ?  No, that doen't clear the flag, but all the others.

Yeah, I was wrong as SCMD_INITIALIZED is included in (actually same as) 
SCMD_PRESERVED_FLAGS mask.

Sorry for the noise.

Cheers,
John

[PATCH 4/8] scsi: remove the cmd field from struct scsi_request

[Christoph Hellwig]

Now that each scsi_request is backed by a scsi_cmnd, there is no
need indirect the CDB storage.  Change all submitters of SCSI
passthrough request to store the CDB information directly in the
scsi_cmnd, and while doing so allocate the full 32 bytes that cover
all Linux supported SCSI hosts instead of requiring dynamic allocation
for > 16 byte CDBs.  On 64-bit systems this does not change the size of
the scsi_cmnd at all, while on 32-bit systems it slightly increases it
for now, but that increase will be made up by the removal of the
remaining scsi_request fields.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/ata/libata-scsi.c           |  4 +++-
 drivers/block/pktcdvd.c             |  6 +++--
 drivers/scsi/scsi_bsg.c             | 23 +++++++++---------
 drivers/scsi/scsi_debugfs.c         |  4 +---
 drivers/scsi/scsi_error.c           | 28 +++++++++++-----------
 drivers/scsi/scsi_ioctl.c           | 37 +++++++++++++----------------
 drivers/scsi/scsi_lib.c             | 19 +++++++--------
 drivers/scsi/scsi_logging.c         |  5 +---
 drivers/scsi/sd.c                   | 28 +---------------------
 drivers/scsi/sg.c                   | 30 +++++++++--------------
 drivers/scsi/sr.c                   | 24 ++++++++++---------
 drivers/scsi/st.c                   | 12 ++++++----
 drivers/scsi/ufs/ufshpb.c           | 22 ++++++++---------
 drivers/target/target_core_pscsi.c  | 12 ++++++----
 drivers/usb/storage/cypress_atacb.c |  1 -
 drivers/usb/storage/isd200.c        |  4 ++--
 include/scsi/scsi_cmnd.h            |  7 +-----
 include/scsi/scsi_eh.h              |  4 +---
 include/scsi/scsi_request.h         | 11 ---------
 19 files changed, 113 insertions(+), 168 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index ed8be585a98f7..c8283bd4ffd15 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1022,7 +1022,9 @@ void ata_scsi_sdev_config(struct scsi_device *sdev)
  */
 bool ata_scsi_dma_need_drain(struct request *rq)
 {
-	return atapi_cmd_type(scsi_req(rq)->cmd[0]) == ATAPI_MISC;
+	struct scsi_cmnd *scmd = blk_mq_rq_to_pdu(rq);
+
+	return atapi_cmd_type(scmd->cmnd[0]) == ATAPI_MISC;
 }
 EXPORT_SYMBOL_GPL(ata_scsi_dma_need_drain);
 
diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index 2b6b70a39e760..42c284b2d7f93 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -693,6 +693,7 @@ static void pkt_rbtree_insert(struct pktcdvd_device *pd, struct pkt_rb_node *nod
 static int pkt_generic_packet(struct pktcdvd_device *pd, struct packet_command *cgc)
 {
 	struct request_queue *q = bdev_get_queue(pd->bdev);
+	struct scsi_cmnd *scmd;
 	struct request *rq;
 	int ret = 0;
 
@@ -700,6 +701,7 @@ static int pkt_generic_packet(struct pktcdvd_device *pd, struct packet_command *
 			     REQ_OP_DRV_OUT : REQ_OP_DRV_IN, 0);
 	if (IS_ERR(rq))
 		return PTR_ERR(rq);
+	scmd = blk_mq_rq_to_pdu(rq);
 
 	if (cgc->buflen) {
 		ret = blk_rq_map_kern(q, rq, cgc->buffer, cgc->buflen,
@@ -708,8 +710,8 @@ static int pkt_generic_packet(struct pktcdvd_device *pd, struct packet_command *
 			goto out;
 	}
 
-	scsi_req(rq)->cmd_len = COMMAND_SIZE(cgc->cmd[0]);
-	memcpy(scsi_req(rq)->cmd, cgc->cmd, CDROM_PACKET_SIZE);
+	scmd->cmd_len = COMMAND_SIZE(cgc->cmd[0]);
+	memcpy(scmd->cmnd, cgc->cmd, CDROM_PACKET_SIZE);
 
 	rq->timeout = 60*HZ;
 	if (cgc->quiet)
diff --git a/drivers/scsi/scsi_bsg.c b/drivers/scsi/scsi_bsg.c
index b7a464383cc0b..f8b65bd75ee10 100644
--- a/drivers/scsi/scsi_bsg.c
+++ b/drivers/scsi/scsi_bsg.c
@@ -13,6 +13,7 @@ static int scsi_bsg_sg_io_fn(struct request_queue *q, struct sg_io_v4 *hdr,
 		fmode_t mode, unsigned int timeout)
 {
 	struct scsi_request *sreq;
+	struct scsi_cmnd *scmd;
 	struct request *rq;
 	struct bio *bio;
 	int ret;
@@ -33,19 +34,19 @@ static int scsi_bsg_sg_io_fn(struct request_queue *q, struct sg_io_v4 *hdr,
 
 	ret = -ENOMEM;
 	sreq = scsi_req(rq);
-	sreq->cmd_len = hdr->request_len;
-	if (sreq->cmd_len > BLK_MAX_CDB) {
-		sreq->cmd = kzalloc(sreq->cmd_len, GFP_KERNEL);
-		if (!sreq->cmd)
-			goto out_put_request;
+	scmd = blk_mq_rq_to_pdu(rq);
+	scmd->cmd_len = hdr->request_len;
+	if (scmd->cmd_len > sizeof(scmd->cmnd)) {
+		ret = -EINVAL;
+		goto out_put_request;
 	}
 
 	ret = -EFAULT;
-	if (copy_from_user(sreq->cmd, uptr64(hdr->request), sreq->cmd_len))
-		goto out_free_cmd;
+	if (copy_from_user(scmd->cmnd, uptr64(hdr->request), scmd->cmd_len))
+		goto out_put_request;
 	ret = -EPERM;
-	if (!scsi_cmd_allowed(sreq->cmd, mode))
-		goto out_free_cmd;
+	if (!scsi_cmd_allowed(scmd->cmnd, mode))
+		goto out_put_request;
 
 	ret = 0;
 	if (hdr->dout_xfer_len) {
@@ -57,7 +58,7 @@ static int scsi_bsg_sg_io_fn(struct request_queue *q, struct sg_io_v4 *hdr,
 	}
 
 	if (ret)
-		goto out_free_cmd;
+		goto out_put_request;
 
 	bio = rq->bio;
 	blk_execute_rq(rq, !(hdr->flags & BSG_FLAG_Q_AT_TAIL));
@@ -92,8 +93,6 @@ static int scsi_bsg_sg_io_fn(struct request_queue *q, struct sg_io_v4 *hdr,
 
 	blk_rq_unmap_user(bio);
 
-out_free_cmd:
-	scsi_req_free_cmd(scsi_req(rq));
 out_put_request:
 	blk_mq_free_request(rq);
 	return ret;
diff --git a/drivers/scsi/scsi_debugfs.c b/drivers/scsi/scsi_debugfs.c
index db8517f1a485a..17d7f73a895c6 100644
--- a/drivers/scsi/scsi_debugfs.c
+++ b/drivers/scsi/scsi_debugfs.c
@@ -36,11 +36,9 @@ void scsi_show_rq(struct seq_file *m, struct request *rq)
 	struct scsi_cmnd *cmd = container_of(scsi_req(rq), typeof(*cmd), req);
 	int alloc_ms = jiffies_to_msecs(jiffies - cmd->jiffies_at_alloc);
 	int timeout_ms = jiffies_to_msecs(rq->timeout);
-	const u8 *const cdb = READ_ONCE(cmd->cmnd);
 	char buf[80] = "(?)";
 
-	if (cdb)
-		__scsi_format_command(buf, sizeof(buf), cdb, cmd->cmd_len);
+	__scsi_format_command(buf, sizeof(buf), cmd->cmnd, cmd->cmd_len);
 	seq_printf(m, ", .cmd=%s, .retries=%d, .result = %#x, .flags=", buf,
 		   cmd->retries, cmd->result);
 	scsi_flags_show(m, cmd->flags, scsi_cmd_flags,
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 60a6ae9d1219f..5383139a3de86 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -980,7 +980,7 @@ static void scsi_abort_eh_cmnd(struct scsi_cmnd *scmd)
  * @scmd:       SCSI command structure to hijack
  * @ses:        structure to save restore information
  * @cmnd:       CDB to send. Can be NULL if no new cmnd is needed
- * @cmnd_size:  size in bytes of @cmnd (must be <= BLK_MAX_CDB)
+ * @cmnd_size:  size in bytes of @cmnd (must be <= MAX_COMMAND_SIZE)
  * @sense_bytes: size of sense data to copy. or 0 (if != 0 @cmnd is ignored)
  *
  * This function is used to save a scsi command information before re-execution
@@ -1002,7 +1002,6 @@ void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct scsi_eh_save *ses,
 	 * command.
 	 */
 	ses->cmd_len = scmd->cmd_len;
-	ses->cmnd = scmd->cmnd;
 	ses->data_direction = scmd->sc_data_direction;
 	ses->sdb = scmd->sdb;
 	ses->result = scmd->result;
@@ -1013,8 +1012,8 @@ void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct scsi_eh_save *ses,
 
 	scmd->prot_op = SCSI_PROT_NORMAL;
 	scmd->eh_eflags = 0;
-	scmd->cmnd = ses->eh_cmnd;
-	memset(scmd->cmnd, 0, BLK_MAX_CDB);
+	memcpy(ses->cmnd, scmd->cmnd, sizeof(ses->cmnd));
+	memset(scmd->cmnd, 0, sizeof(scmd->cmnd));
 	memset(&scmd->sdb, 0, sizeof(scmd->sdb));
 	scmd->result = 0;
 	scmd->req.resid_len = 0;
@@ -1033,7 +1032,7 @@ void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct scsi_eh_save *ses,
 	} else {
 		scmd->sc_data_direction = DMA_NONE;
 		if (cmnd) {
-			BUG_ON(cmnd_size > BLK_MAX_CDB);
+			BUG_ON(cmnd_size > sizeof(scmd->cmnd));
 			memcpy(scmd->cmnd, cmnd, cmnd_size);
 			scmd->cmd_len = COMMAND_SIZE(scmd->cmnd[0]);
 		}
@@ -1066,7 +1065,7 @@ void scsi_eh_restore_cmnd(struct scsi_cmnd* scmd, struct scsi_eh_save *ses)
 	 * Restore original data
 	 */
 	scmd->cmd_len = ses->cmd_len;
-	scmd->cmnd = ses->cmnd;
+	memcpy(scmd->cmnd, ses->cmnd, sizeof(ses->cmnd));
 	scmd->sc_data_direction = ses->data_direction;
 	scmd->sdb = ses->sdb;
 	scmd->result = ses->result;
@@ -2022,6 +2021,7 @@ static void eh_lock_door_done(struct request *req, blk_status_t status)
  */
 static void scsi_eh_lock_door(struct scsi_device *sdev)
 {
+	struct scsi_cmnd *scmd;
 	struct request *req;
 	struct scsi_request *rq;
 
@@ -2029,14 +2029,15 @@ static void scsi_eh_lock_door(struct scsi_device *sdev)
 	if (IS_ERR(req))
 		return;
 	rq = scsi_req(req);
+	scmd = blk_mq_rq_to_pdu(req);
 
-	rq->cmd[0] = ALLOW_MEDIUM_REMOVAL;
-	rq->cmd[1] = 0;
-	rq->cmd[2] = 0;
-	rq->cmd[3] = 0;
-	rq->cmd[4] = SCSI_REMOVAL_PREVENT;
-	rq->cmd[5] = 0;
-	rq->cmd_len = COMMAND_SIZE(rq->cmd[0]);
+	scmd->cmnd[0] = ALLOW_MEDIUM_REMOVAL;
+	scmd->cmnd[1] = 0;
+	scmd->cmnd[2] = 0;
+	scmd->cmnd[3] = 0;
+	scmd->cmnd[4] = SCSI_REMOVAL_PREVENT;
+	scmd->cmnd[5] = 0;
+	scmd->cmd_len = COMMAND_SIZE(scmd->cmnd[0]);
 
 	req->rq_flags |= RQF_QUIET;
 	req->timeout = 10 * HZ;
@@ -2399,7 +2400,6 @@ scsi_ioctl_reset(struct scsi_device *dev, int __user *arg)
 
 	scmd = (struct scsi_cmnd *)(rq + 1);
 	scsi_init_command(dev, scmd);
-	scmd->cmnd = scsi_req(rq)->cmd;
 
 	scmd->submitter = SUBMITTED_BY_SCSI_RESET_IOCTL;
 	memset(&scmd->sdb, 0, sizeof(scmd->sdb));
diff --git a/drivers/scsi/scsi_ioctl.c b/drivers/scsi/scsi_ioctl.c
index e13fd380deb6f..6d9f1b44566d1 100644
--- a/drivers/scsi/scsi_ioctl.c
+++ b/drivers/scsi/scsi_ioctl.c
@@ -345,19 +345,15 @@ EXPORT_SYMBOL(scsi_cmd_allowed);
 static int scsi_fill_sghdr_rq(struct scsi_device *sdev, struct request *rq,
 		struct sg_io_hdr *hdr, fmode_t mode)
 {
-	struct scsi_request *req = scsi_req(rq);
+	struct scsi_cmnd *scmd = blk_mq_rq_to_pdu(rq);
 
 	if (hdr->cmd_len < 6)
 		return -EMSGSIZE;
-	if (copy_from_user(req->cmd, hdr->cmdp, hdr->cmd_len))
+	if (copy_from_user(scmd->cmnd, hdr->cmdp, hdr->cmd_len))
 		return -EFAULT;
-	if (!scsi_cmd_allowed(req->cmd, mode))
+	if (!scsi_cmd_allowed(scmd->cmnd, mode))
 		return -EPERM;
-
-	/*
-	 * fill in request structure
-	 */
-	req->cmd_len = hdr->cmd_len;
+	scmd->cmd_len = hdr->cmd_len;
 
 	rq->timeout = msecs_to_jiffies(hdr->timeout);
 	if (!rq->timeout)
@@ -416,6 +412,7 @@ static int sg_io(struct scsi_device *sdev, struct sg_io_hdr *hdr, fmode_t mode)
 	int at_head = 0;
 	struct request *rq;
 	struct scsi_request *req;
+	struct scsi_cmnd *scmd;
 	struct bio *bio;
 
 	if (hdr->interface_id != 'S')
@@ -444,16 +441,16 @@ static int sg_io(struct scsi_device *sdev, struct sg_io_hdr *hdr, fmode_t mode)
 	if (IS_ERR(rq))
 		return PTR_ERR(rq);
 	req = scsi_req(rq);
+	scmd = blk_mq_rq_to_pdu(rq);
 
-	if (hdr->cmd_len > BLK_MAX_CDB) {
-		req->cmd = kzalloc(hdr->cmd_len, GFP_KERNEL);
-		if (!req->cmd)
-			goto out_put_request;
+	if (hdr->cmd_len > sizeof(scmd->cmnd)) {
+		ret = -EINVAL;
+		goto out_put_request;
 	}
 
 	ret = scsi_fill_sghdr_rq(sdev, rq, hdr, mode);
 	if (ret < 0)
-		goto out_free_cdb;
+		goto out_put_request;
 
 	ret = 0;
 	if (hdr->iovec_count) {
@@ -463,7 +460,7 @@ static int sg_io(struct scsi_device *sdev, struct sg_io_hdr *hdr, fmode_t mode)
 		ret = import_iovec(rq_data_dir(rq), hdr->dxferp,
 				   hdr->iovec_count, 0, &iov, &i);
 		if (ret < 0)
-			goto out_free_cdb;
+			goto out_put_request;
 
 		/* SG_IO howto says that the shorter of the two wins */
 		iov_iter_truncate(&i, hdr->dxfer_len);
@@ -475,7 +472,7 @@ static int sg_io(struct scsi_device *sdev, struct sg_io_hdr *hdr, fmode_t mode)
 				      hdr->dxfer_len, GFP_KERNEL);
 
 	if (ret)
-		goto out_free_cdb;
+		goto out_put_request;
 
 	bio = rq->bio;
 	req->retries = 0;
@@ -488,8 +485,6 @@ static int sg_io(struct scsi_device *sdev, struct sg_io_hdr *hdr, fmode_t mode)
 
 	ret = scsi_complete_sghdr_rq(rq, hdr, bio);
 
-out_free_cdb:
-	scsi_req_free_cmd(req);
 out_put_request:
 	blk_mq_free_request(rq);
 	return ret;
@@ -530,6 +525,7 @@ static int sg_scsi_ioctl(struct request_queue *q, fmode_t mode,
 	struct scsi_request *req;
 	int err;
 	unsigned int in_len, out_len, bytes, opcode, cmdlen;
+	struct scsi_cmnd *scmd;
 	char *buffer = NULL;
 
 	if (!sic)
@@ -561,6 +557,7 @@ static int sg_scsi_ioctl(struct request_queue *q, fmode_t mode,
 		goto error_free_buffer;
 	}
 	req = scsi_req(rq);
+	scmd = blk_mq_rq_to_pdu(rq);
 
 	cmdlen = COMMAND_SIZE(opcode);
 
@@ -568,15 +565,15 @@ static int sg_scsi_ioctl(struct request_queue *q, fmode_t mode,
 	 * get command and data to send to device, if any
 	 */
 	err = -EFAULT;
-	req->cmd_len = cmdlen;
-	if (copy_from_user(req->cmd, sic->data, cmdlen))
+	scmd->cmd_len = cmdlen;
+	if (copy_from_user(scmd->cmnd, sic->data, cmdlen))
 		goto error;
 
 	if (in_len && copy_from_user(buffer, sic->data + cmdlen, in_len))
 		goto error;
 
 	err = -EPERM;
-	if (!scsi_cmd_allowed(req->cmd, mode))
+	if (!scsi_cmd_allowed(scmd->cmnd, mode))
 		goto error;
 
 	/* default.  possible overridden later */
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 960795d469d8c..be02999b78ea5 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -214,6 +214,7 @@ int __scsi_execute(struct scsi_device *sdev, const unsigned char *cmd,
 {
 	struct request *req;
 	struct scsi_request *rq;
+	struct scsi_cmnd *scmd;
 	int ret;
 
 	req = scsi_alloc_request(sdev->request_queue,
@@ -231,8 +232,9 @@ int __scsi_execute(struct scsi_device *sdev, const unsigned char *cmd,
 		if (ret)
 			goto out;
 	}
-	rq->cmd_len = COMMAND_SIZE(cmd[0]);
-	memcpy(rq->cmd, cmd, rq->cmd_len);
+	scmd = blk_mq_rq_to_pdu(req);
+	scmd->cmd_len = COMMAND_SIZE(cmd[0]);
+	memcpy(scmd->cmnd, cmd, scmd->cmd_len);
 	rq->retries = retries;
 	req->timeout = timeout;
 	req->cmd_flags |= flags;
@@ -1126,9 +1128,9 @@ static void scsi_initialize_rq(struct request *rq)
 	struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq);
 	struct scsi_request *req = &cmd->req;
 
-	memset(req->__cmd, 0, sizeof(req->__cmd));
-	req->cmd = req->__cmd;
-	req->cmd_len = BLK_MAX_CDB;
+	memset(cmd->cmnd, 0, sizeof(cmd->cmnd));
+	cmd->cmd_len = MAX_COMMAND_SIZE;
+
 	req->sense_len = 0;
 
 	init_rcu_head(&cmd->rcu);
@@ -1197,8 +1199,6 @@ static blk_status_t scsi_setup_scsi_cmnd(struct scsi_device *sdev,
 		memset(&cmd->sdb, 0, sizeof(cmd->sdb));
 	}
 
-	cmd->cmd_len = scsi_req(req)->cmd_len;
-	cmd->cmnd = scsi_req(req)->cmd;
 	cmd->transfersize = blk_rq_bytes(req);
 	cmd->allowed = scsi_req(req)->retries;
 	return BLK_STS_OK;
@@ -1568,8 +1568,6 @@ static blk_status_t scsi_prepare_cmd(struct request *req)
 	cmd->prot_type = 0;
 	cmd->prot_flags = 0;
 	cmd->submitter = 0;
-	cmd->cmd_len = 0;
-	cmd->cmnd = NULL;
 	memset(&cmd->sdb, 0, sizeof(cmd->sdb));
 	cmd->underflow = 0;
 	cmd->transfersize = 0;
@@ -1617,8 +1615,7 @@ static blk_status_t scsi_prepare_cmd(struct request *req)
 			return ret;
 	}
 
-	cmd->cmnd = scsi_req(req)->cmd = scsi_req(req)->__cmd;
-	memset(cmd->cmnd, 0, BLK_MAX_CDB);
+	memset(cmd->cmnd, 0, sizeof(cmd->cmnd));
 	return scsi_cmd_to_driver(cmd)->init_command(cmd);
 }
 
diff --git a/drivers/scsi/scsi_logging.c b/drivers/scsi/scsi_logging.c
index 1f8f80b2dbfcb..ff89de86545d1 100644
--- a/drivers/scsi/scsi_logging.c
+++ b/drivers/scsi/scsi_logging.c
@@ -87,7 +87,7 @@ void scmd_printk(const char *level, const struct scsi_cmnd *scmd,
 	char *logbuf;
 	size_t off = 0, logbuf_len;
 
-	if (!scmd || !scmd->cmnd)
+	if (!scmd)
 		return;
 
 	logbuf = scsi_log_reserve_buffer(&logbuf_len);
@@ -183,9 +183,6 @@ void scsi_print_command(struct scsi_cmnd *cmd)
 	char *logbuf;
 	size_t off, logbuf_len;
 
-	if (!cmd->cmnd)
-		return;
-
 	logbuf = scsi_log_reserve_buffer(&logbuf_len);
 	if (!logbuf)
 		return;
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index bf32a05414276..2f9d160bc8c21 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -128,7 +128,6 @@ static DEFINE_IDA(sd_index_ida);
 static DEFINE_MUTEX(sd_ref_mutex);
 
 static struct kmem_cache *sd_cdb_cache;
-static mempool_t *sd_cdb_pool;
 static mempool_t *sd_page_pool;
 static struct lock_class_key sd_bio_compl_lkclass;
 
@@ -1075,13 +1074,7 @@ static blk_status_t sd_setup_rw32_cmnd(struct scsi_cmnd *cmd, bool write,
 				       sector_t lba, unsigned int nr_blocks,
 				       unsigned char flags)
 {
-	cmd->cmnd = mempool_alloc(sd_cdb_pool, GFP_ATOMIC);
-	if (unlikely(cmd->cmnd == NULL))
-		return BLK_STS_RESOURCE;
-
 	cmd->cmd_len = SD_EXT_CDB_SIZE;
-	memset(cmd->cmnd, 0, cmd->cmd_len);
-
 	cmd->cmnd[0]  = VARIABLE_LENGTH_CMD;
 	cmd->cmnd[7]  = 0x18; /* Additional CDB len */
 	cmd->cmnd[9]  = write ? WRITE_32 : READ_32;
@@ -1313,17 +1306,9 @@ static blk_status_t sd_init_command(struct scsi_cmnd *cmd)
 static void sd_uninit_command(struct scsi_cmnd *SCpnt)
 {
 	struct request *rq = scsi_cmd_to_rq(SCpnt);
-	u8 *cmnd;
 
 	if (rq->rq_flags & RQF_SPECIAL_PAYLOAD)
 		mempool_free(rq->special_vec.bv_page, sd_page_pool);
-
-	if (SCpnt->cmnd != scsi_req(rq)->cmd) {
-		cmnd = SCpnt->cmnd;
-		SCpnt->cmnd = NULL;
-		SCpnt->cmd_len = 0;
-		mempool_free(cmnd, sd_cdb_pool);
-	}
 }
 
 static bool sd_need_revalidate(struct block_device *bdev,
@@ -3819,18 +3804,11 @@ static int __init init_sd(void)
 		goto err_out_class;
 	}
 
-	sd_cdb_pool = mempool_create_slab_pool(SD_MEMPOOL_SIZE, sd_cdb_cache);
-	if (!sd_cdb_pool) {
-		printk(KERN_ERR "sd: can't init extended cdb pool\n");
-		err = -ENOMEM;
-		goto err_out_cache;
-	}
-
 	sd_page_pool = mempool_create_page_pool(SD_MEMPOOL_SIZE, 0);
 	if (!sd_page_pool) {
 		printk(KERN_ERR "sd: can't init discard page pool\n");
 		err = -ENOMEM;
-		goto err_out_ppool;
+		goto err_out_cache;
 	}
 
 	err = scsi_register_driver(&sd_template.gendrv);
@@ -3842,9 +3820,6 @@ static int __init init_sd(void)
 err_out_driver:
 	mempool_destroy(sd_page_pool);
 
-err_out_ppool:
-	mempool_destroy(sd_cdb_pool);
-
 err_out_cache:
 	kmem_cache_destroy(sd_cdb_cache);
 
@@ -3868,7 +3843,6 @@ static void __exit exit_sd(void)
 	SCSI_LOG_HLQUEUE(3, printk("exit_sd: exiting sd driver\n"));
 
 	scsi_unregister_driver(&sd_template.gendrv);
-	mempool_destroy(sd_cdb_pool);
 	mempool_destroy(sd_page_pool);
 	kmem_cache_destroy(sd_cdb_cache);
 
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index bbd75026ec937..cc3f11270dc23 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -818,7 +818,6 @@ sg_common_write(Sg_fd * sfp, Sg_request * srp,
 	}
 	if (atomic_read(&sdp->detaching)) {
 		if (srp->bio) {
-			scsi_req_free_cmd(scsi_req(srp->rq));
 			blk_mq_free_request(srp->rq);
 			srp->rq = NULL;
 		}
@@ -1393,7 +1392,6 @@ sg_rq_end_io(struct request *rq, blk_status_t status)
 	 * blk_rq_unmap_user() can be called from user context.
 	 */
 	srp->rq = NULL;
-	scsi_req_free_cmd(scsi_req(rq));
 	blk_mq_free_request(rq);
 
 	write_lock_irqsave(&sfp->rq_list_lock, iflags);
@@ -1738,18 +1736,12 @@ sg_start_req(Sg_request *srp, unsigned char *cmd)
 	struct request_queue *q = sfp->parentdp->device->request_queue;
 	struct rq_map_data *md, map_data;
 	int rw = hp->dxfer_direction == SG_DXFER_TO_DEV ? WRITE : READ;
-	unsigned char *long_cmdp = NULL;
+	struct scsi_cmnd *scmd;
 
 	SCSI_LOG_TIMEOUT(4, sg_printk(KERN_INFO, sfp->parentdp,
 				      "sg_start_req: dxfer_len=%d\n",
 				      dxfer_len));
 
-	if (hp->cmd_len > BLK_MAX_CDB) {
-		long_cmdp = kzalloc(hp->cmd_len, GFP_KERNEL);
-		if (!long_cmdp)
-			return -ENOMEM;
-	}
-
 	/*
 	 * NOTE
 	 *
@@ -1763,16 +1755,18 @@ sg_start_req(Sg_request *srp, unsigned char *cmd)
 	 */
 	rq = scsi_alloc_request(q, hp->dxfer_direction == SG_DXFER_TO_DEV ?
 			REQ_OP_DRV_OUT : REQ_OP_DRV_IN, 0);
-	if (IS_ERR(rq)) {
-		kfree(long_cmdp);
+	if (IS_ERR(rq))
 		return PTR_ERR(rq);
-	}
+	scmd = blk_mq_rq_to_pdu(rq);
 	req = scsi_req(rq);
 
-	if (hp->cmd_len > BLK_MAX_CDB)
-		req->cmd = long_cmdp;
-	memcpy(req->cmd, cmd, hp->cmd_len);
-	req->cmd_len = hp->cmd_len;
+	if (hp->cmd_len > sizeof(scmd->cmnd)) {
+		blk_mq_free_request(rq);
+		return -EINVAL;
+	}
+
+	memcpy(scmd->cmnd, cmd, hp->cmd_len);
+	scmd->cmd_len = hp->cmd_len;
 
 	srp->rq = rq;
 	rq->end_io_data = srp;
@@ -1865,10 +1859,8 @@ sg_finish_rem_req(Sg_request *srp)
 	if (srp->bio)
 		ret = blk_rq_unmap_user(srp->bio);
 
-	if (srp->rq) {
-		scsi_req_free_cmd(scsi_req(srp->rq));
+	if (srp->rq)
 		blk_mq_free_request(srp->rq);
-	}
 
 	if (srp->res_used)
 		sg_unlink_reserve(sfp, srp);
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index f925b1f1f9ada..3adba389cae51 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -966,6 +966,7 @@ static int sr_read_cdda_bpc(struct cdrom_device_info *cdi, void __user *ubuf,
 	struct gendisk *disk = cdi->disk;
 	u32 len = nr * CD_FRAMESIZE_RAW;
 	struct scsi_request *req;
+	struct scsi_cmnd *scmd;
 	struct request *rq;
 	struct bio *bio;
 	int ret;
@@ -974,22 +975,23 @@ static int sr_read_cdda_bpc(struct cdrom_device_info *cdi, void __user *ubuf,
 	if (IS_ERR(rq))
 		return PTR_ERR(rq);
 	req = scsi_req(rq);
+	scmd = blk_mq_rq_to_pdu(rq);
 
 	ret = blk_rq_map_user(disk->queue, rq, NULL, ubuf, len, GFP_KERNEL);
 	if (ret)
 		goto out_put_request;
 
-	req->cmd[0] = GPCMD_READ_CD;
-	req->cmd[1] = 1 << 2;
-	req->cmd[2] = (lba >> 24) & 0xff;
-	req->cmd[3] = (lba >> 16) & 0xff;
-	req->cmd[4] = (lba >>  8) & 0xff;
-	req->cmd[5] = lba & 0xff;
-	req->cmd[6] = (nr >> 16) & 0xff;
-	req->cmd[7] = (nr >>  8) & 0xff;
-	req->cmd[8] = nr & 0xff;
-	req->cmd[9] = 0xf8;
-	req->cmd_len = 12;
+	scmd->cmnd[0] = GPCMD_READ_CD;
+	scmd->cmnd[1] = 1 << 2;
+	scmd->cmnd[2] = (lba >> 24) & 0xff;
+	scmd->cmnd[3] = (lba >> 16) & 0xff;
+	scmd->cmnd[4] = (lba >>  8) & 0xff;
+	scmd->cmnd[5] = lba & 0xff;
+	scmd->cmnd[6] = (nr >> 16) & 0xff;
+	scmd->cmnd[7] = (nr >>  8) & 0xff;
+	scmd->cmnd[8] = nr & 0xff;
+	scmd->cmnd[9] = 0xf8;
+	scmd->cmd_len = 12;
 	rq->timeout = 60 * HZ;
 	bio = rq->bio;
 
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index e869e90e05afe..229e819a1797e 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -472,10 +472,11 @@ static void st_release_request(struct st_request *streq)
 
 static void st_do_stats(struct scsi_tape *STp, struct request *req)
 {
+	struct scsi_cmnd *scmd = blk_mq_rq_to_pdu(req);
 	ktime_t now;
 
 	now = ktime_get();
-	if (scsi_req(req)->cmd[0] == WRITE_6) {
+	if (scmd->cmnd[0] == WRITE_6) {
 		now = ktime_sub(now, STp->stats->write_time);
 		atomic64_add(ktime_to_ns(now), &STp->stats->tot_write_time);
 		atomic64_add(ktime_to_ns(now), &STp->stats->tot_io_time);
@@ -489,7 +490,7 @@ static void st_do_stats(struct scsi_tape *STp, struct request *req)
 		} else
 			atomic64_add(atomic_read(&STp->stats->last_write_size),
 				&STp->stats->write_byte_cnt);
-	} else if (scsi_req(req)->cmd[0] == READ_6) {
+	} else if (scmd->cmnd[0] == READ_6) {
 		now = ktime_sub(now, STp->stats->read_time);
 		atomic64_add(ktime_to_ns(now), &STp->stats->tot_read_time);
 		atomic64_add(ktime_to_ns(now), &STp->stats->tot_io_time);
@@ -542,12 +543,14 @@ static int st_scsi_execute(struct st_request *SRpnt, const unsigned char *cmd,
 	struct rq_map_data *mdata = &SRpnt->stp->buffer->map_data;
 	int err = 0;
 	struct scsi_tape *STp = SRpnt->stp;
+	struct scsi_cmnd *scmd;
 
 	req = scsi_alloc_request(SRpnt->stp->device->request_queue,
 			data_direction == DMA_TO_DEVICE ?
 			REQ_OP_DRV_OUT : REQ_OP_DRV_IN, 0);
 	if (IS_ERR(req))
 		return PTR_ERR(req);
+	scmd = blk_mq_rq_to_pdu(req);
 	rq = scsi_req(req);
 	req->rq_flags |= RQF_QUIET;
 
@@ -574,9 +577,8 @@ static int st_scsi_execute(struct st_request *SRpnt, const unsigned char *cmd,
 	}
 
 	SRpnt->bio = req->bio;
-	rq->cmd_len = COMMAND_SIZE(cmd[0]);
-	memset(rq->cmd, 0, BLK_MAX_CDB);
-	memcpy(rq->cmd, cmd, rq->cmd_len);
+	scmd->cmd_len = COMMAND_SIZE(cmd[0]);
+	memcpy(scmd->cmnd, cmd, scmd->cmd_len);
 	req->timeout = timeout;
 	rq->retries = retries;
 	req->end_io_data = SRpnt;
diff --git a/drivers/scsi/ufs/ufshpb.c b/drivers/scsi/ufs/ufshpb.c
index 2d36a0715fca6..3ca745ad616c1 100644
--- a/drivers/scsi/ufs/ufshpb.c
+++ b/drivers/scsi/ufs/ufshpb.c
@@ -666,15 +666,14 @@ static void ufshpb_execute_umap_req(struct ufshpb_lu *hpb,
 				   struct ufshpb_req *umap_req,
 				   struct ufshpb_region *rgn)
 {
-	struct request *req;
-	struct scsi_request *rq;
+	struct request *req = umap_req->req;
+	struct scsi_cmnd *scmd = blk_mq_rq_to_pdu(req);
 
-	req = umap_req->req;
 	req->timeout = 0;
-	req->end_io_data = (void *)umap_req;
-	rq = scsi_req(req);
-	ufshpb_set_unmap_cmd(rq->cmd, rgn);
-	rq->cmd_len = HPB_WRITE_BUFFER_CMD_LENGTH;
+	req->end_io_data = umap_req;
+
+	ufshpb_set_unmap_cmd(scmd->cmnd, rgn);
+	scmd->cmd_len = HPB_WRITE_BUFFER_CMD_LENGTH;
 
 	blk_execute_rq_nowait(req, true, ufshpb_umap_req_compl_fn);
 
@@ -686,7 +685,7 @@ static int ufshpb_execute_map_req(struct ufshpb_lu *hpb,
 {
 	struct request_queue *q;
 	struct request *req;
-	struct scsi_request *rq;
+	struct scsi_cmnd *scmd;
 	int mem_size = hpb->srgn_mem_size;
 	int ret = 0;
 	int i;
@@ -709,14 +708,13 @@ static int ufshpb_execute_map_req(struct ufshpb_lu *hpb,
 
 	req->end_io_data = map_req;
 
-	rq = scsi_req(req);
-
 	if (unlikely(last))
 		mem_size = hpb->last_srgn_entries * HPB_ENTRY_SIZE;
 
-	ufshpb_set_read_buf_cmd(rq->cmd, map_req->rb.rgn_idx,
+	scmd = blk_mq_rq_to_pdu(req);
+	ufshpb_set_read_buf_cmd(scmd->cmnd, map_req->rb.rgn_idx,
 				map_req->rb.srgn_idx, mem_size);
-	rq->cmd_len = HPB_READ_BUFFER_CMD_LENGTH;
+	scmd->cmd_len = HPB_READ_BUFFER_CMD_LENGTH;
 
 	blk_execute_rq_nowait(req, true, ufshpb_map_req_compl_fn);
 
diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c
index 622158ed5d9ce..0a8078db923d3 100644
--- a/drivers/target/target_core_pscsi.c
+++ b/drivers/target/target_core_pscsi.c
@@ -961,6 +961,7 @@ pscsi_execute_cmd(struct se_cmd *cmd)
 	struct scatterlist *sgl = cmd->t_data_sg;
 	u32 sgl_nents = cmd->t_data_nents;
 	struct pscsi_dev_virt *pdv = PSCSI_DEV(cmd->se_dev);
+	struct scsi_cmnd *scmd;
 	struct request *req;
 	sense_reason_t ret;
 
@@ -978,12 +979,15 @@ pscsi_execute_cmd(struct se_cmd *cmd)
 
 	req->end_io = pscsi_req_done;
 	req->end_io_data = cmd;
-	scsi_req(req)->cmd_len = scsi_command_size(cmd->t_task_cdb);
-	if (scsi_req(req)->cmd_len > BLK_MAX_CDB) {
+
+	scmd = blk_mq_rq_to_pdu(req);
+	scmd->cmd_len = scsi_command_size(cmd->t_task_cdb);
+	if (scmd->cmd_len > sizeof(scmd->cmnd)) {
 		ret = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
 		goto fail_put_request;
 	}
-	memcpy(scsi_req(req)->cmd, cmd->t_task_cdb, scsi_req(req)->cmd_len);
+	memcpy(scmd->cmnd, cmd->t_task_cdb, scmd->cmd_len);
+
 	if (pdv->pdv_sd->type == TYPE_DISK ||
 	    pdv->pdv_sd->type == TYPE_ZBC)
 		req->timeout = PS_TIMEOUT_DISK;
@@ -991,7 +995,7 @@ pscsi_execute_cmd(struct se_cmd *cmd)
 		req->timeout = PS_TIMEOUT_OTHER;
 	scsi_req(req)->retries = PS_RETRY;
 
-	cmd->priv = scsi_req(req)->cmd;
+	cmd->priv = scmd->cmnd;
 
 	blk_execute_rq_nowait(req, cmd->sam_task_attr == TCM_HEAD_TAG,
 			pscsi_req_done);
diff --git a/drivers/usb/storage/cypress_atacb.c b/drivers/usb/storage/cypress_atacb.c
index 2f7093ba5a2f4..98b3ec352a138 100644
--- a/drivers/usb/storage/cypress_atacb.c
+++ b/drivers/usb/storage/cypress_atacb.c
@@ -177,7 +177,6 @@ static void cypress_atacb_passthrough(struct scsi_cmnd *srb, struct us_data *us)
 		 * but reading register selected in srb->cmnd[4]
 		 */
 		srb->cmd_len = 16;
-		srb->cmnd = ses.cmnd;
 		srb->cmnd[2] = 1;
 
 		usb_stor_transparent_scsi_command(srb, us);
diff --git a/drivers/usb/storage/isd200.c b/drivers/usb/storage/isd200.c
index 3c76336e43bb2..05429f1f69f9d 100644
--- a/drivers/usb/storage/isd200.c
+++ b/drivers/usb/storage/isd200.c
@@ -326,7 +326,7 @@ struct isd200_info {
 
 	/* maximum number of LUNs supported */
 	unsigned char MaxLUNs;
-	unsigned char cmnd[BLK_MAX_CDB];
+	unsigned char cmnd[MAX_COMMAND_SIZE];
 	struct scsi_cmnd srb;
 	struct scatterlist sg;
 };
@@ -485,7 +485,7 @@ static int isd200_action( struct us_data *us, int action,
 	int status;
 
 	memset(&ata, 0, sizeof(ata));
-	srb->cmnd = info->cmnd;
+	memcpy(srb->cmnd, info->cmnd, MAX_COMMAND_SIZE);
 	srb->device = &srb_dev;
 
 	ata.generic.SignatureByte0 = info->ConfigData.ATAMajorCommand;
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 7a19c8bbaed9e..3e432e25645ac 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -28,9 +28,6 @@ struct scsi_driver;
  * supports without specifying a cmd_len by ULD's
  */
 #define MAX_COMMAND_SIZE 16
-#if (MAX_COMMAND_SIZE > BLK_MAX_CDB)
-# error MAX_COMMAND_SIZE can not be bigger than BLK_MAX_CDB
-#endif
 
 struct scsi_data_buffer {
 	struct sg_table table;
@@ -100,9 +97,7 @@ struct scsi_cmnd {
 	unsigned short cmd_len;
 	enum dma_data_direction sc_data_direction;
 
-	/* These elements define the operation we are about to perform */
-	unsigned char *cmnd;
-
+	unsigned char cmnd[32]; /* SCSI CDB */
 
 	/* These elements define the operation we ultimately want to perform */
 	struct scsi_data_buffer sdb;
diff --git a/include/scsi/scsi_eh.h b/include/scsi/scsi_eh.h
index 468094254b3cc..1ae08e81339fa 100644
--- a/include/scsi/scsi_eh.h
+++ b/include/scsi/scsi_eh.h
@@ -38,10 +38,8 @@ struct scsi_eh_save {
 	unsigned underflow;
 	unsigned char cmd_len;
 	unsigned char prot_op;
-	unsigned char *cmnd;
+	unsigned char cmnd[32];
 	struct scsi_data_buffer sdb;
-	/* new command support */
-	unsigned char eh_cmnd[BLK_MAX_CDB];
 	struct scatterlist sense_sgl;
 };
 
diff --git a/include/scsi/scsi_request.h b/include/scsi/scsi_request.h
index 9129b23e12bc0..aeee0611bcbe1 100644
--- a/include/scsi/scsi_request.h
+++ b/include/scsi/scsi_request.h
@@ -4,12 +4,7 @@
 
 #include <linux/blk-mq.h>
 
-#define BLK_MAX_CDB	16
-
 struct scsi_request {
-	unsigned char	__cmd[BLK_MAX_CDB];
-	unsigned char	*cmd;
-	unsigned short	cmd_len;
 	int		result;
 	unsigned int	sense_len;
 	unsigned int	resid_len;	/* residual count */
@@ -22,10 +17,4 @@ static inline struct scsi_request *scsi_req(struct request *rq)
 	return blk_mq_rq_to_pdu(rq);
 }
 
-static inline void scsi_req_free_cmd(struct scsi_request *req)
-{
-	if (req->cmd != req->__cmd)
-		kfree(req->cmd);
-}
-
 #endif /* _SCSI_SCSI_REQUEST_H */
-- 
2.30.2

Re: [PATCH 4/8] scsi: remove the cmd field from struct scsi_request

[Martin K. Petersen]

Christoph,

> Change all submitters of SCSI passthrough request to store the CDB
> information directly in the scsi_cmnd, and while doing so allocate the
> full 32 bytes that cover all Linux supported SCSI hosts instead of
> requiring dynamic allocation for > 16 byte CDBs.

Been wanting to get rid of that for so long!

> On 64-bit systems this does not change the size of the scsi_cmnd at
> all, while on 32-bit systems it slightly increases it for now, but
> that increase will be made up by the removal of the remaining
> scsi_request fields.

Excellent!

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 4/8] scsi: remove the cmd field from struct scsi_request

[Bart Van Assche]

On 2/22/22 06:04, Christoph Hellwig wrote:
> Now that each scsi_request is backed by a scsi_cmnd, there is no
> need indirect the CDB storage.  Change all submitters of SCSI
       ^
missing verb?
> passthrough request to store the CDB information directly in the
               ^^^^^^^
               requests?

Anyway:

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

[PATCH 5/8] scsi: remove the sense and sense_len fields from struct scsi_request

[Christoph Hellwig]

Just use the sense_buffer field in struct scsi_cmnd for the sense data
and move the sense_len field over to struct scsi_cmnd.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/scsi_bsg.c            |  7 ++++---
 drivers/scsi/scsi_ioctl.c          | 15 ++++++++-------
 drivers/scsi/scsi_lib.c            | 18 +++++++-----------
 drivers/scsi/sg.c                  |  7 ++++---
 drivers/scsi/sr.c                  |  2 +-
 drivers/scsi/st.c                  |  5 +++--
 drivers/target/target_core_pscsi.c |  3 ++-
 include/scsi/scsi_cmnd.h           |  1 +
 include/scsi/scsi_request.h        |  2 --
 9 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/drivers/scsi/scsi_bsg.c b/drivers/scsi/scsi_bsg.c
index f8b65bd75ee10..0a6f6140501be 100644
--- a/drivers/scsi/scsi_bsg.c
+++ b/drivers/scsi/scsi_bsg.c
@@ -76,11 +76,12 @@ static int scsi_bsg_sg_io_fn(struct request_queue *q, struct sg_io_v4 *hdr,
 		hdr->info |= SG_INFO_CHECK;
 	hdr->response_len = 0;
 
-	if (sreq->sense_len && hdr->response) {
+	if (scmd->sense_len && hdr->response) {
 		int len = min_t(unsigned int, hdr->max_response_len,
-					sreq->sense_len);
+				scmd->sense_len);
 
-		if (copy_to_user(uptr64(hdr->response), sreq->sense, len))
+		if (copy_to_user(uptr64(hdr->response), scmd->sense_buffer,
+				 len))
 			ret = -EFAULT;
 		else
 			hdr->response_len = len;
diff --git a/drivers/scsi/scsi_ioctl.c b/drivers/scsi/scsi_ioctl.c
index 6d9f1b44566d1..731232e6ee664 100644
--- a/drivers/scsi/scsi_ioctl.c
+++ b/drivers/scsi/scsi_ioctl.c
@@ -369,6 +369,7 @@ static int scsi_fill_sghdr_rq(struct scsi_device *sdev, struct request *rq,
 static int scsi_complete_sghdr_rq(struct request *rq, struct sg_io_hdr *hdr,
 		struct bio *bio)
 {
+	struct scsi_cmnd *scmd = blk_mq_rq_to_pdu(rq);
 	struct scsi_request *req = scsi_req(rq);
 	int r, ret = 0;
 
@@ -388,10 +389,10 @@ static int scsi_complete_sghdr_rq(struct request *rq, struct sg_io_hdr *hdr,
 	hdr->resid = req->resid_len;
 	hdr->sb_len_wr = 0;
 
-	if (req->sense_len && hdr->sbp) {
-		int len = min((unsigned int) hdr->mx_sb_len, req->sense_len);
+	if (scmd->sense_len && hdr->sbp) {
+		int len = min((unsigned int) hdr->mx_sb_len, scmd->sense_len);
 
-		if (!copy_to_user(hdr->sbp, req->sense, len))
+		if (!copy_to_user(hdr->sbp, scmd->sense_buffer, len))
 			hdr->sb_len_wr = len;
 		else
 			ret = -EFAULT;
@@ -613,10 +614,10 @@ static int sg_scsi_ioctl(struct request_queue *q, fmode_t mode,
 
 	err = req->result & 0xff;	/* only 8 bit SCSI status */
 	if (err) {
-		if (req->sense_len && req->sense) {
-			bytes = (OMAX_SB_LEN > req->sense_len) ?
-				req->sense_len : OMAX_SB_LEN;
-			if (copy_to_user(sic->data, req->sense, bytes))
+		if (scmd->sense_len && scmd->sense_buffer) {
+			bytes = (OMAX_SB_LEN > scmd->sense_len) ?
+				scmd->sense_len : OMAX_SB_LEN;
+			if (copy_to_user(sic->data, scmd->sense_buffer, bytes))
 				err = -EFAULT;
 		}
 	} else {
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index be02999b78ea5..db269f1d70b83 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -256,10 +256,11 @@ int __scsi_execute(struct scsi_device *sdev, const unsigned char *cmd,
 
 	if (resid)
 		*resid = rq->resid_len;
-	if (sense && rq->sense_len)
-		memcpy(sense, rq->sense, SCSI_SENSE_BUFFERSIZE);
+	if (sense && scmd->sense_len)
+		memcpy(sense, scmd->sense_buffer, SCSI_SENSE_BUFFERSIZE);
 	if (sshdr)
-		scsi_normalize_sense(rq->sense, rq->sense_len, sshdr);
+		scsi_normalize_sense(scmd->sense_buffer, scmd->sense_len,
+				     sshdr);
 	ret = rq->result;
  out:
 	blk_mq_free_request(req);
@@ -876,9 +877,8 @@ static int scsi_io_completion_nz_result(struct scsi_cmnd *cmd, int result,
 			/*
 			 * SG_IO wants current and deferred errors
 			 */
-			scsi_req(req)->sense_len =
-				min(8 + cmd->sense_buffer[7],
-				    SCSI_SENSE_BUFFERSIZE);
+			cmd->sense_len = min(8 + cmd->sense_buffer[7],
+					     SCSI_SENSE_BUFFERSIZE);
 		}
 		if (sense_current)
 			*blk_statp = scsi_result_to_blk_status(cmd, result);
@@ -1126,13 +1126,10 @@ EXPORT_SYMBOL(scsi_alloc_sgtables);
 static void scsi_initialize_rq(struct request *rq)
 {
 	struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq);
-	struct scsi_request *req = &cmd->req;
 
 	memset(cmd->cmnd, 0, sizeof(cmd->cmnd));
 	cmd->cmd_len = MAX_COMMAND_SIZE;
-
-	req->sense_len = 0;
-
+	cmd->sense_len = 0;
 	init_rcu_head(&cmd->rcu);
 	cmd->jiffies_at_alloc = jiffies;
 	cmd->retries = 0;
@@ -1825,7 +1822,6 @@ static int scsi_mq_init_request(struct blk_mq_tag_set *set, struct request *rq,
 		kmem_cache_alloc_node(scsi_sense_cache, GFP_KERNEL, numa_node);
 	if (!cmd->sense_buffer)
 		return -ENOMEM;
-	cmd->req.sense = cmd->sense_buffer;
 
 	if (scsi_host_get_prot(shost)) {
 		sg = (void *)cmd + sizeof(struct scsi_cmnd) +
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index cc3f11270dc23..0f96c7cde9e51 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -1323,6 +1323,7 @@ sg_rq_end_io_usercontext(struct work_struct *work)
 static void
 sg_rq_end_io(struct request *rq, blk_status_t status)
 {
+	struct scsi_cmnd *scmd = blk_mq_rq_to_pdu(rq);
 	struct sg_request *srp = rq->end_io_data;
 	struct scsi_request *req = scsi_req(rq);
 	Sg_device *sdp;
@@ -1343,7 +1344,7 @@ sg_rq_end_io(struct request *rq, blk_status_t status)
 	if (unlikely(atomic_read(&sdp->detaching)))
 		pr_info("%s: device detaching\n", __func__);
 
-	sense = req->sense;
+	sense = scmd->sense_buffer;
 	result = req->result;
 	resid = req->resid_len;
 
@@ -1380,8 +1381,8 @@ sg_rq_end_io(struct request *rq, blk_status_t status)
 		}
 	}
 
-	if (req->sense_len)
-		memcpy(srp->sense_b, req->sense, SCSI_SENSE_BUFFERSIZE);
+	if (scmd->sense_len)
+		memcpy(srp->sense_b, scmd->sense_buffer, SCSI_SENSE_BUFFERSIZE);
 
 	/* Rely on write phase to clean out srp status values, so no "else" */
 
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 3adba389cae51..1d19dd13d7f01 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -999,7 +999,7 @@ static int sr_read_cdda_bpc(struct cdrom_device_info *cdi, void __user *ubuf,
 	if (scsi_req(rq)->result) {
 		struct scsi_sense_hdr sshdr;
 
-		scsi_normalize_sense(req->sense, req->sense_len,
+		scsi_normalize_sense(scmd->sense_buffer, scmd->sense_len,
 				     &sshdr);
 		*last_sense = sshdr.sense_key;
 		ret = -EIO;
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index 229e819a1797e..9b98e848d78c1 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -514,6 +514,7 @@ static void st_do_stats(struct scsi_tape *STp, struct request *req)
 
 static void st_scsi_execute_end(struct request *req, blk_status_t status)
 {
+	struct scsi_cmnd *scmd = blk_mq_rq_to_pdu(req);
 	struct st_request *SRpnt = req->end_io_data;
 	struct scsi_request *rq = scsi_req(req);
 	struct scsi_tape *STp = SRpnt->stp;
@@ -525,8 +526,8 @@ static void st_scsi_execute_end(struct request *req, blk_status_t status)
 	st_do_stats(STp, req);
 
 	tmp = SRpnt->bio;
-	if (rq->sense_len)
-		memcpy(SRpnt->sense, rq->sense, SCSI_SENSE_BUFFERSIZE);
+	if (scmd->sense_len)
+		memcpy(SRpnt->sense, scmd->sense_buffer, SCSI_SENSE_BUFFERSIZE);
 	if (SRpnt->waiting)
 		complete(SRpnt->waiting);
 
diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c
index 0a8078db923d3..9146193d0576a 100644
--- a/drivers/target/target_core_pscsi.c
+++ b/drivers/target/target_core_pscsi.c
@@ -1031,6 +1031,7 @@ static sector_t pscsi_get_blocks(struct se_device *dev)
 static void pscsi_req_done(struct request *req, blk_status_t status)
 {
 	struct se_cmd *cmd = req->end_io_data;
+	struct scsi_cmnd *scmd = blk_mq_rq_to_pdu(req);
 	int result = scsi_req(req)->result;
 	enum sam_status scsi_status = result & 0xff;
 	u8 *cdb = cmd->priv;
@@ -1040,7 +1041,7 @@ static void pscsi_req_done(struct request *req, blk_status_t status)
 			" 0x%02x Result: 0x%08x\n", cmd, cdb[0], result);
 	}
 
-	pscsi_complete_cmd(cmd, scsi_status, scsi_req(req)->sense);
+	pscsi_complete_cmd(cmd, scsi_status, scmd->sense_buffer);
 
 	switch (host_byte(result)) {
 	case DID_OK:
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 3e432e25645ac..47add5b32f460 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -112,6 +112,7 @@ struct scsi_cmnd {
 				   reconnects.   Probably == sector
 				   size */
 
+	unsigned sense_len;
 	unsigned char *sense_buffer;
 				/* obtained by REQUEST SENSE when
 				 * CHECK CONDITION is received on original
diff --git a/include/scsi/scsi_request.h b/include/scsi/scsi_request.h
index aeee0611bcbe1..bed1cc49132ab 100644
--- a/include/scsi/scsi_request.h
+++ b/include/scsi/scsi_request.h
@@ -6,10 +6,8 @@
 
 struct scsi_request {
 	int		result;
-	unsigned int	sense_len;
 	unsigned int	resid_len;	/* residual count */
 	int		retries;
-	void		*sense;
 };
 
 static inline struct scsi_request *scsi_req(struct request *rq)
-- 
2.30.2

Re: [PATCH 5/8] scsi: remove the sense and sense_len fields from struct scsi_request

[Bart Van Assche]

On 2/22/22 06:04, Christoph Hellwig wrote:
> @@ -613,10 +614,10 @@ static int sg_scsi_ioctl(struct request_queue *q, fmode_t mode,
>   
>   	err = req->result & 0xff;	/* only 8 bit SCSI status */
>   	if (err) {
> -		if (req->sense_len && req->sense) {
> -			bytes = (OMAX_SB_LEN > req->sense_len) ?
> -				req->sense_len : OMAX_SB_LEN;
> -			if (copy_to_user(sic->data, req->sense, bytes))
> +		if (scmd->sense_len && scmd->sense_buffer) {
> +			bytes = (OMAX_SB_LEN > scmd->sense_len) ?
> +				scmd->sense_len : OMAX_SB_LEN;
> +			if (copy_to_user(sic->data, scmd->sense_buffer, bytes))
>   				err = -EFAULT;
>   		}
>   	} else {

This change would be a good opportunity to remove the two superfluous 
parentheses from the above code.

> diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
> index 3e432e25645ac..47add5b32f460 100644
> --- a/include/scsi/scsi_cmnd.h
> +++ b/include/scsi/scsi_cmnd.h
> @@ -112,6 +112,7 @@ struct scsi_cmnd {
>   				   reconnects.   Probably == sector
>   				   size */
>   
> +	unsigned sense_len;
>   	unsigned char *sense_buffer;
>   				/* obtained by REQUEST SENSE when
>   				 * CHECK CONDITION is received on original

Isn't "unsigned int" preferred over "unsigned" in new code?

Anyway:

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

Re: [PATCH 5/8] scsi: remove the sense and sense_len fields from struct scsi_request

[Christoph Hellwig]

On Wed, Feb 23, 2022 at 03:39:58PM -0800, Bart Van Assche wrote:
> On 2/22/22 06:04, Christoph Hellwig wrote:
>> @@ -613,10 +614,10 @@ static int sg_scsi_ioctl(struct request_queue *q, fmode_t mode,
>>     	err = req->result & 0xff;	/* only 8 bit SCSI status */
>>   	if (err) {
>> -		if (req->sense_len && req->sense) {
>> -			bytes = (OMAX_SB_LEN > req->sense_len) ?
>> -				req->sense_len : OMAX_SB_LEN;
>> -			if (copy_to_user(sic->data, req->sense, bytes))
>> +		if (scmd->sense_len && scmd->sense_buffer) {
>> +			bytes = (OMAX_SB_LEN > scmd->sense_len) ?
>> +				scmd->sense_len : OMAX_SB_LEN;
>> +			if (copy_to_user(sic->data, scmd->sense_buffer, bytes))
>>   				err = -EFAULT;
>>   		}
>>   	} else {
>
> This change would be a good opportunity to remove the two superfluous 
> parentheses from the above code.

Or switch to use the min or min_t macro, yes.

>
>> diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
>> index 3e432e25645ac..47add5b32f460 100644
>> --- a/include/scsi/scsi_cmnd.h
>> +++ b/include/scsi/scsi_cmnd.h
>> @@ -112,6 +112,7 @@ struct scsi_cmnd {
>>   				   reconnects.   Probably == sector
>>   				   size */
>>   +	unsigned sense_len;
>>   	unsigned char *sense_buffer;
>>   				/* obtained by REQUEST SENSE when
>>   				 * CHECK CONDITION is received on original
>
> Isn't "unsigned int" preferred over "unsigned" in new code?

checkpatch states that, but for absolutely no reason.

[PATCH 6/8] scsi: move the resid_len field from struct scsi_request to struct scsi_cmnd

[Christoph Hellwig]

Prepare for removing the scsi_request structure by moving the resid_len
field to struct scsi_cmnd.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/scsi_bsg.c            | 4 ++--
 drivers/scsi/scsi_error.c          | 6 +++---
 drivers/scsi/scsi_ioctl.c          | 2 +-
 drivers/scsi/scsi_lib.c            | 6 +++---
 drivers/scsi/sg.c                  | 2 +-
 drivers/scsi/st.c                  | 2 +-
 drivers/target/target_core_pscsi.c | 2 +-
 include/scsi/scsi_cmnd.h           | 6 +++---
 include/scsi/scsi_request.h        | 1 -
 9 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/drivers/scsi/scsi_bsg.c b/drivers/scsi/scsi_bsg.c
index 0a6f6140501be..4c697d0ddf1d2 100644
--- a/drivers/scsi/scsi_bsg.c
+++ b/drivers/scsi/scsi_bsg.c
@@ -88,9 +88,9 @@ static int scsi_bsg_sg_io_fn(struct request_queue *q, struct sg_io_v4 *hdr,
 	}
 
 	if (rq_data_dir(rq) == READ)
-		hdr->din_resid = sreq->resid_len;
+		hdr->din_resid = scmd->resid_len;
 	else
-		hdr->dout_resid = sreq->resid_len;
+		hdr->dout_resid = scmd->resid_len;
 
 	blk_rq_unmap_user(bio);
 
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 5383139a3de86..cf02d1c503897 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -1005,7 +1005,7 @@ void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct scsi_eh_save *ses,
 	ses->data_direction = scmd->sc_data_direction;
 	ses->sdb = scmd->sdb;
 	ses->result = scmd->result;
-	ses->resid_len = scmd->req.resid_len;
+	ses->resid_len = scmd->resid_len;
 	ses->underflow = scmd->underflow;
 	ses->prot_op = scmd->prot_op;
 	ses->eh_eflags = scmd->eh_eflags;
@@ -1016,7 +1016,7 @@ void scsi_eh_prep_cmnd(struct scsi_cmnd *scmd, struct scsi_eh_save *ses,
 	memset(scmd->cmnd, 0, sizeof(scmd->cmnd));
 	memset(&scmd->sdb, 0, sizeof(scmd->sdb));
 	scmd->result = 0;
-	scmd->req.resid_len = 0;
+	scmd->resid_len = 0;
 
 	if (sense_bytes) {
 		scmd->sdb.length = min_t(unsigned, SCSI_SENSE_BUFFERSIZE,
@@ -1069,7 +1069,7 @@ void scsi_eh_restore_cmnd(struct scsi_cmnd* scmd, struct scsi_eh_save *ses)
 	scmd->sc_data_direction = ses->data_direction;
 	scmd->sdb = ses->sdb;
 	scmd->result = ses->result;
-	scmd->req.resid_len = ses->resid_len;
+	scmd->resid_len = ses->resid_len;
 	scmd->underflow = ses->underflow;
 	scmd->prot_op = ses->prot_op;
 	scmd->eh_eflags = ses->eh_eflags;
diff --git a/drivers/scsi/scsi_ioctl.c b/drivers/scsi/scsi_ioctl.c
index 731232e6ee664..546b98321023c 100644
--- a/drivers/scsi/scsi_ioctl.c
+++ b/drivers/scsi/scsi_ioctl.c
@@ -386,7 +386,7 @@ static int scsi_complete_sghdr_rq(struct request *rq, struct sg_io_hdr *hdr,
 	hdr->info = 0;
 	if (hdr->masked_status || hdr->host_status || hdr->driver_status)
 		hdr->info |= SG_INFO_CHECK;
-	hdr->resid = req->resid_len;
+	hdr->resid = scmd->resid_len;
 	hdr->sb_len_wr = 0;
 
 	if (scmd->sense_len && hdr->sbp) {
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index db269f1d70b83..c0664f7a86e73 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -251,11 +251,11 @@ int __scsi_execute(struct scsi_device *sdev, const unsigned char *cmd,
 	 * is invalid.  Prevent the garbage from being misinterpreted
 	 * and prevent security leaks by zeroing out the excess data.
 	 */
-	if (unlikely(rq->resid_len > 0 && rq->resid_len <= bufflen))
-		memset(buffer + (bufflen - rq->resid_len), 0, rq->resid_len);
+	if (unlikely(scmd->resid_len > 0 && scmd->resid_len <= bufflen))
+		memset(buffer + bufflen - scmd->resid_len, 0, scmd->resid_len);
 
 	if (resid)
-		*resid = rq->resid_len;
+		*resid = scmd->resid_len;
 	if (sense && scmd->sense_len)
 		memcpy(sense, scmd->sense_buffer, SCSI_SENSE_BUFFERSIZE);
 	if (sshdr)
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 0f96c7cde9e51..3eaabfb315e0c 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -1346,7 +1346,7 @@ sg_rq_end_io(struct request *rq, blk_status_t status)
 
 	sense = scmd->sense_buffer;
 	result = req->result;
-	resid = req->resid_len;
+	resid = scmd->resid_len;
 
 	SCSI_LOG_TIMEOUT(4, sg_printk(KERN_INFO, sdp,
 				      "sg_cmd_done: pack_id=%d, res=0x%x\n",
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index 9b98e848d78c1..0546d2c84ad16 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -521,7 +521,7 @@ static void st_scsi_execute_end(struct request *req, blk_status_t status)
 	struct bio *tmp;
 
 	STp->buffer->cmdstat.midlevel_result = SRpnt->result = rq->result;
-	STp->buffer->cmdstat.residual = rq->resid_len;
+	STp->buffer->cmdstat.residual = scmd->resid_len;
 
 	st_do_stats(STp, req);
 
diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c
index 9146193d0576a..d5828da3d392a 100644
--- a/drivers/target/target_core_pscsi.c
+++ b/drivers/target/target_core_pscsi.c
@@ -1046,7 +1046,7 @@ static void pscsi_req_done(struct request *req, blk_status_t status)
 	switch (host_byte(result)) {
 	case DID_OK:
 		target_complete_cmd_with_length(cmd, scsi_status,
-			cmd->data_length - scsi_req(req)->resid_len);
+			cmd->data_length - scmd->resid_len);
 		break;
 	default:
 		pr_debug("PSCSI Host Byte exception at cmd: %p CDB:"
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 47add5b32f460..5ff0a6e8460c3 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -111,7 +111,7 @@ struct scsi_cmnd {
 				   (ie, between disconnect / 
 				   reconnects.   Probably == sector
 				   size */
-
+	unsigned resid_len;	/* residual count */
 	unsigned sense_len;
 	unsigned char *sense_buffer;
 				/* obtained by REQUEST SENSE when
@@ -200,12 +200,12 @@ static inline unsigned scsi_bufflen(struct scsi_cmnd *cmd)
 
 static inline void scsi_set_resid(struct scsi_cmnd *cmd, unsigned int resid)
 {
-	cmd->req.resid_len = resid;
+	cmd->resid_len = resid;
 }
 
 static inline unsigned int scsi_get_resid(struct scsi_cmnd *cmd)
 {
-	return cmd->req.resid_len;
+	return cmd->resid_len;
 }
 
 #define scsi_for_each_sg(cmd, sg, nseg, __i)			\
diff --git a/include/scsi/scsi_request.h b/include/scsi/scsi_request.h
index bed1cc49132ab..74be75336a54d 100644
--- a/include/scsi/scsi_request.h
+++ b/include/scsi/scsi_request.h
@@ -6,7 +6,6 @@
 
 struct scsi_request {
 	int		result;
-	unsigned int	resid_len;	/* residual count */
 	int		retries;
 };
 
-- 
2.30.2

Re: [PATCH 6/8] scsi: move the resid_len field from struct scsi_request to struct scsi_cmnd

[Bart Van Assche]

On 2/22/22 06:04, Christoph Hellwig wrote:
> diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
> index 47add5b32f460..5ff0a6e8460c3 100644
> --- a/include/scsi/scsi_cmnd.h
> +++ b/include/scsi/scsi_cmnd.h
> @@ -111,7 +111,7 @@ struct scsi_cmnd {
>   				   (ie, between disconnect /
>   				   reconnects.   Probably == sector
>   				   size */
> -
> +	unsigned resid_len;	/* residual count */
>   	unsigned sense_len;
>   	unsigned char *sense_buffer;
>   				/* obtained by REQUEST SENSE when

Same comment here about "unsigned" vs. "unsigned int". Anyway:

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

[PATCH 7/8] scsi: move the result field from struct scsi_request to struct scsi_cmnd

[Christoph Hellwig]

Prepare for removing the scsi_request structure by moving the result
field to struct scsi_cmnd.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/pktcdvd.c            |  2 +-
 drivers/scsi/scsi_bsg.c            |  8 +++-----
 drivers/scsi/scsi_ioctl.c          |  9 ++++-----
 drivers/scsi/scsi_lib.c            | 15 ++++-----------
 drivers/scsi/sg.c                  |  3 +--
 drivers/scsi/sr.c                  |  2 +-
 drivers/scsi/st.c                  |  7 +++----
 drivers/target/target_core_pscsi.c |  9 ++++-----
 include/scsi/scsi_request.h        |  1 -
 9 files changed, 21 insertions(+), 35 deletions(-)

diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index 42c284b2d7f93..aca94ebf49478 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -718,7 +718,7 @@ static int pkt_generic_packet(struct pktcdvd_device *pd, struct packet_command *
 		rq->rq_flags |= RQF_QUIET;
 
 	blk_execute_rq(rq, false);
-	if (scsi_req(rq)->result)
+	if (scmd->result)
 		ret = -EIO;
 out:
 	blk_mq_free_request(rq);
diff --git a/drivers/scsi/scsi_bsg.c b/drivers/scsi/scsi_bsg.c
index 4c697d0ddf1d2..8039c3c11a6ee 100644
--- a/drivers/scsi/scsi_bsg.c
+++ b/drivers/scsi/scsi_bsg.c
@@ -12,7 +12,6 @@
 static int scsi_bsg_sg_io_fn(struct request_queue *q, struct sg_io_v4 *hdr,
 		fmode_t mode, unsigned int timeout)
 {
-	struct scsi_request *sreq;
 	struct scsi_cmnd *scmd;
 	struct request *rq;
 	struct bio *bio;
@@ -33,7 +32,6 @@ static int scsi_bsg_sg_io_fn(struct request_queue *q, struct sg_io_v4 *hdr,
 	rq->timeout = timeout;
 
 	ret = -ENOMEM;
-	sreq = scsi_req(rq);
 	scmd = blk_mq_rq_to_pdu(rq);
 	scmd->cmd_len = hdr->request_len;
 	if (scmd->cmd_len > sizeof(scmd->cmnd)) {
@@ -66,10 +64,10 @@ static int scsi_bsg_sg_io_fn(struct request_queue *q, struct sg_io_v4 *hdr,
 	/*
 	 * fill in all the output members
 	 */
-	hdr->device_status = sreq->result & 0xff;
-	hdr->transport_status = host_byte(sreq->result);
+	hdr->device_status = scmd->result & 0xff;
+	hdr->transport_status = host_byte(scmd->result);
 	hdr->driver_status = 0;
-	if (scsi_status_is_check_condition(sreq->result))
+	if (scsi_status_is_check_condition(scmd->result))
 		hdr->driver_status = DRIVER_SENSE;
 	hdr->info = 0;
 	if (hdr->device_status || hdr->transport_status || hdr->driver_status)
diff --git a/drivers/scsi/scsi_ioctl.c b/drivers/scsi/scsi_ioctl.c
index 546b98321023c..e95e104103f13 100644
--- a/drivers/scsi/scsi_ioctl.c
+++ b/drivers/scsi/scsi_ioctl.c
@@ -370,16 +370,15 @@ static int scsi_complete_sghdr_rq(struct request *rq, struct sg_io_hdr *hdr,
 		struct bio *bio)
 {
 	struct scsi_cmnd *scmd = blk_mq_rq_to_pdu(rq);
-	struct scsi_request *req = scsi_req(rq);
 	int r, ret = 0;
 
 	/*
 	 * fill in all the output members
 	 */
-	hdr->status = req->result & 0xff;
-	hdr->masked_status = status_byte(req->result);
+	hdr->status = scmd->result & 0xff;
+	hdr->masked_status = status_byte(scmd->result);
 	hdr->msg_status = COMMAND_COMPLETE;
-	hdr->host_status = host_byte(req->result);
+	hdr->host_status = host_byte(scmd->result);
 	hdr->driver_status = 0;
 	if (scsi_status_is_check_condition(hdr->status))
 		hdr->driver_status = DRIVER_SENSE;
@@ -612,7 +611,7 @@ static int sg_scsi_ioctl(struct request_queue *q, fmode_t mode,
 
 	blk_execute_rq(rq, false);
 
-	err = req->result & 0xff;	/* only 8 bit SCSI status */
+	err = scmd->result & 0xff;	/* only 8 bit SCSI status */
 	if (err) {
 		if (scmd->sense_len && scmd->sense_buffer) {
 			bytes = (OMAX_SB_LEN > scmd->sense_len) ?
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index c0664f7a86e73..786bd989ca4de 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -261,7 +261,7 @@ int __scsi_execute(struct scsi_device *sdev, const unsigned char *cmd,
 	if (sshdr)
 		scsi_normalize_sense(scmd->sense_buffer, scmd->sense_len,
 				     sshdr);
-	ret = rq->result;
+	ret = scmd->result;
  out:
 	blk_mq_free_request(req);
 
@@ -959,13 +959,6 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 	if (unlikely(result))	/* a nz result may or may not be an error */
 		result = scsi_io_completion_nz_result(cmd, result, &blk_stat);
 
-	if (unlikely(blk_rq_is_passthrough(req))) {
-		/*
-		 * scsi_result_to_blk_status may have reset the host_byte
-		 */
-		scsi_req(req)->result = cmd->result;
-	}
-
 	/*
 	 * Next deal with any sectors which we were able to correctly
 	 * handle.
@@ -1780,15 +1773,15 @@ static blk_status_t scsi_queue_rq(struct blk_mq_hw_ctx *hctx,
 			ret = BLK_STS_DEV_RESOURCE;
 		break;
 	case BLK_STS_AGAIN:
-		scsi_req(req)->result = DID_BUS_BUSY << 16;
+		cmd->result = DID_BUS_BUSY << 16;
 		if (req->rq_flags & RQF_DONTPREP)
 			scsi_mq_uninit_cmd(cmd);
 		break;
 	default:
 		if (unlikely(!scsi_device_online(sdev)))
-			scsi_req(req)->result = DID_NO_CONNECT << 16;
+			cmd->result = DID_NO_CONNECT << 16;
 		else
-			scsi_req(req)->result = DID_ERROR << 16;
+			cmd->result = DID_ERROR << 16;
 		/*
 		 * Make sure to release all allocated resources when
 		 * we hit an error, as we will never see this command
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 3eaabfb315e0c..26a753521cb29 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -1325,7 +1325,6 @@ sg_rq_end_io(struct request *rq, blk_status_t status)
 {
 	struct scsi_cmnd *scmd = blk_mq_rq_to_pdu(rq);
 	struct sg_request *srp = rq->end_io_data;
-	struct scsi_request *req = scsi_req(rq);
 	Sg_device *sdp;
 	Sg_fd *sfp;
 	unsigned long iflags;
@@ -1345,7 +1344,7 @@ sg_rq_end_io(struct request *rq, blk_status_t status)
 		pr_info("%s: device detaching\n", __func__);
 
 	sense = scmd->sense_buffer;
-	result = req->result;
+	result = scmd->result;
 	resid = scmd->resid_len;
 
 	SCSI_LOG_TIMEOUT(4, sg_printk(KERN_INFO, sdp,
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 1d19dd13d7f01..494d00b05f53e 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -996,7 +996,7 @@ static int sr_read_cdda_bpc(struct cdrom_device_info *cdi, void __user *ubuf,
 	bio = rq->bio;
 
 	blk_execute_rq(rq, false);
-	if (scsi_req(rq)->result) {
+	if (scmd->result) {
 		struct scsi_sense_hdr sshdr;
 
 		scsi_normalize_sense(scmd->sense_buffer, scmd->sense_len,
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index 0546d2c84ad16..c8533ca225bc0 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -481,7 +481,7 @@ static void st_do_stats(struct scsi_tape *STp, struct request *req)
 		atomic64_add(ktime_to_ns(now), &STp->stats->tot_write_time);
 		atomic64_add(ktime_to_ns(now), &STp->stats->tot_io_time);
 		atomic64_inc(&STp->stats->write_cnt);
-		if (scsi_req(req)->result) {
+		if (scmd->result) {
 			atomic64_add(atomic_read(&STp->stats->last_write_size)
 				- STp->buffer->cmdstat.residual,
 				&STp->stats->write_byte_cnt);
@@ -495,7 +495,7 @@ static void st_do_stats(struct scsi_tape *STp, struct request *req)
 		atomic64_add(ktime_to_ns(now), &STp->stats->tot_read_time);
 		atomic64_add(ktime_to_ns(now), &STp->stats->tot_io_time);
 		atomic64_inc(&STp->stats->read_cnt);
-		if (scsi_req(req)->result) {
+		if (scmd->result) {
 			atomic64_add(atomic_read(&STp->stats->last_read_size)
 				- STp->buffer->cmdstat.residual,
 				&STp->stats->read_byte_cnt);
@@ -516,11 +516,10 @@ static void st_scsi_execute_end(struct request *req, blk_status_t status)
 {
 	struct scsi_cmnd *scmd = blk_mq_rq_to_pdu(req);
 	struct st_request *SRpnt = req->end_io_data;
-	struct scsi_request *rq = scsi_req(req);
 	struct scsi_tape *STp = SRpnt->stp;
 	struct bio *tmp;
 
-	STp->buffer->cmdstat.midlevel_result = SRpnt->result = rq->result;
+	STp->buffer->cmdstat.midlevel_result = SRpnt->result = scmd->result;
 	STp->buffer->cmdstat.residual = scmd->resid_len;
 
 	st_do_stats(STp, req);
diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c
index d5828da3d392a..5b23a0ff905ea 100644
--- a/drivers/target/target_core_pscsi.c
+++ b/drivers/target/target_core_pscsi.c
@@ -1032,25 +1032,24 @@ static void pscsi_req_done(struct request *req, blk_status_t status)
 {
 	struct se_cmd *cmd = req->end_io_data;
 	struct scsi_cmnd *scmd = blk_mq_rq_to_pdu(req);
-	int result = scsi_req(req)->result;
-	enum sam_status scsi_status = result & 0xff;
+	enum sam_status scsi_status = scmd->result & 0xff;
 	u8 *cdb = cmd->priv;
 
 	if (scsi_status != SAM_STAT_GOOD) {
 		pr_debug("PSCSI Status Byte exception at cmd: %p CDB:"
-			" 0x%02x Result: 0x%08x\n", cmd, cdb[0], result);
+			" 0x%02x Result: 0x%08x\n", cmd, cdb[0], scmd->result);
 	}
 
 	pscsi_complete_cmd(cmd, scsi_status, scmd->sense_buffer);
 
-	switch (host_byte(result)) {
+	switch (host_byte(scmd->result)) {
 	case DID_OK:
 		target_complete_cmd_with_length(cmd, scsi_status,
 			cmd->data_length - scmd->resid_len);
 		break;
 	default:
 		pr_debug("PSCSI Host Byte exception at cmd: %p CDB:"
-			" 0x%02x Result: 0x%08x\n", cmd, cdb[0], result);
+			" 0x%02x Result: 0x%08x\n", cmd, cdb[0], scmd->result);
 		target_complete_cmd(cmd, SAM_STAT_CHECK_CONDITION);
 		break;
 	}
diff --git a/include/scsi/scsi_request.h b/include/scsi/scsi_request.h
index 74be75336a54d..929c7bd5c72fe 100644
--- a/include/scsi/scsi_request.h
+++ b/include/scsi/scsi_request.h
@@ -5,7 +5,6 @@
 #include <linux/blk-mq.h>
 
 struct scsi_request {
-	int		result;
 	int		retries;
 };
 
-- 
2.30.2

Re: [PATCH 7/8] scsi: move the result field from struct scsi_request to struct scsi_cmnd

[Bart Van Assche]

On 2/22/22 06:04, Christoph Hellwig wrote:
> Prepare for removing the scsi_request structure by moving the result
> field to struct scsi_cmnd.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

[PATCH 8/8] scsi: remove struct scsi_request

[Christoph Hellwig]

Let submitters initialized the scmd->allowed field directly instead of
indirecting through struct scsi_request and remove the now superflous
structure.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/cdrom/cdrom.c              |  1 -
 drivers/scsi/scsi_debugfs.c        |  2 +-
 drivers/scsi/scsi_error.c          |  4 +---
 drivers/scsi/scsi_ioctl.c          | 12 ++++--------
 drivers/scsi/scsi_lib.c            |  6 +-----
 drivers/scsi/scsi_transport_sas.c  |  1 -
 drivers/scsi/sg.c                  |  4 +---
 drivers/scsi/sr.c                  |  2 --
 drivers/scsi/st.c                  |  4 +---
 drivers/target/target_core_pscsi.c |  2 +-
 include/scsi/scsi_cmnd.h           |  2 --
 include/scsi/scsi_request.h        | 16 ----------------
 12 files changed, 10 insertions(+), 46 deletions(-)
 delete mode 100644 include/scsi/scsi_request.h

diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
index 1b57d4666e43c..7bd10d63ddbe5 100644
--- a/drivers/cdrom/cdrom.c
+++ b/drivers/cdrom/cdrom.c
@@ -284,7 +284,6 @@
 #include <linux/times.h>
 #include <linux/uaccess.h>
 #include <scsi/scsi_common.h>
-#include <scsi/scsi_request.h>
 
 /* used to tell the module to turn on full debugging messages */
 static bool debug;
diff --git a/drivers/scsi/scsi_debugfs.c b/drivers/scsi/scsi_debugfs.c
index 17d7f73a895c6..217b70c678c3a 100644
--- a/drivers/scsi/scsi_debugfs.c
+++ b/drivers/scsi/scsi_debugfs.c
@@ -33,7 +33,7 @@ static int scsi_flags_show(struct seq_file *m, const unsigned long flags,
 
 void scsi_show_rq(struct seq_file *m, struct request *rq)
 {
-	struct scsi_cmnd *cmd = container_of(scsi_req(rq), typeof(*cmd), req);
+	struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq);
 	int alloc_ms = jiffies_to_msecs(jiffies - cmd->jiffies_at_alloc);
 	int timeout_ms = jiffies_to_msecs(rq->timeout);
 	char buf[80] = "(?)";
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index cf02d1c503897..9c237b223e63c 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -2023,12 +2023,10 @@ static void scsi_eh_lock_door(struct scsi_device *sdev)
 {
 	struct scsi_cmnd *scmd;
 	struct request *req;
-	struct scsi_request *rq;
 
 	req = scsi_alloc_request(sdev->request_queue, REQ_OP_DRV_IN, 0);
 	if (IS_ERR(req))
 		return;
-	rq = scsi_req(req);
 	scmd = blk_mq_rq_to_pdu(req);
 
 	scmd->cmnd[0] = ALLOW_MEDIUM_REMOVAL;
@@ -2041,7 +2039,7 @@ static void scsi_eh_lock_door(struct scsi_device *sdev)
 
 	req->rq_flags |= RQF_QUIET;
 	req->timeout = 10 * HZ;
-	rq->retries = 5;
+	scmd->allowed = 5;
 
 	blk_execute_rq_nowait(req, true, eh_lock_door_done);
 }
diff --git a/drivers/scsi/scsi_ioctl.c b/drivers/scsi/scsi_ioctl.c
index e95e104103f13..2b066dc7f9336 100644
--- a/drivers/scsi/scsi_ioctl.c
+++ b/drivers/scsi/scsi_ioctl.c
@@ -411,7 +411,6 @@ static int sg_io(struct scsi_device *sdev, struct sg_io_hdr *hdr, fmode_t mode)
 	int writing = 0;
 	int at_head = 0;
 	struct request *rq;
-	struct scsi_request *req;
 	struct scsi_cmnd *scmd;
 	struct bio *bio;
 
@@ -440,7 +439,6 @@ static int sg_io(struct scsi_device *sdev, struct sg_io_hdr *hdr, fmode_t mode)
 			     REQ_OP_DRV_OUT : REQ_OP_DRV_IN, 0);
 	if (IS_ERR(rq))
 		return PTR_ERR(rq);
-	req = scsi_req(rq);
 	scmd = blk_mq_rq_to_pdu(rq);
 
 	if (hdr->cmd_len > sizeof(scmd->cmnd)) {
@@ -475,7 +473,7 @@ static int sg_io(struct scsi_device *sdev, struct sg_io_hdr *hdr, fmode_t mode)
 		goto out_put_request;
 
 	bio = rq->bio;
-	req->retries = 0;
+	scmd->allowed = 0;
 
 	start_time = jiffies;
 
@@ -522,7 +520,6 @@ static int sg_scsi_ioctl(struct request_queue *q, fmode_t mode,
 {
 	enum { OMAX_SB_LEN = 16 };	/* For backward compatibility */
 	struct request *rq;
-	struct scsi_request *req;
 	int err;
 	unsigned int in_len, out_len, bytes, opcode, cmdlen;
 	struct scsi_cmnd *scmd;
@@ -556,7 +553,6 @@ static int sg_scsi_ioctl(struct request_queue *q, fmode_t mode,
 		err = PTR_ERR(rq);
 		goto error_free_buffer;
 	}
-	req = scsi_req(rq);
 	scmd = blk_mq_rq_to_pdu(rq);
 
 	cmdlen = COMMAND_SIZE(opcode);
@@ -577,13 +573,13 @@ static int sg_scsi_ioctl(struct request_queue *q, fmode_t mode,
 		goto error;
 
 	/* default.  possible overridden later */
-	req->retries = 5;
+	scmd->allowed = 5;
 
 	switch (opcode) {
 	case SEND_DIAGNOSTIC:
 	case FORMAT_UNIT:
 		rq->timeout = FORMAT_UNIT_TIMEOUT;
-		req->retries = 1;
+		scmd->allowed = 1;
 		break;
 	case START_STOP:
 		rq->timeout = START_STOP_TIMEOUT;
@@ -596,7 +592,7 @@ static int sg_scsi_ioctl(struct request_queue *q, fmode_t mode,
 		break;
 	case READ_DEFECT_DATA:
 		rq->timeout = READ_DEFECT_DATA_TIMEOUT;
-		req->retries = 1;
+		scmd->allowed = 1;
 		break;
 	default:
 		rq->timeout = BLK_DEFAULT_SG_TIMEOUT;
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 786bd989ca4de..69cea48a0cbe8 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -213,7 +213,6 @@ int __scsi_execute(struct scsi_device *sdev, const unsigned char *cmd,
 		 int *resid)
 {
 	struct request *req;
-	struct scsi_request *rq;
 	struct scsi_cmnd *scmd;
 	int ret;
 
@@ -224,8 +223,6 @@ int __scsi_execute(struct scsi_device *sdev, const unsigned char *cmd,
 	if (IS_ERR(req))
 		return PTR_ERR(req);
 
-	rq = scsi_req(req);
-
 	if (bufflen) {
 		ret = blk_rq_map_kern(sdev->request_queue, req,
 				      buffer, bufflen, GFP_NOIO);
@@ -235,7 +232,7 @@ int __scsi_execute(struct scsi_device *sdev, const unsigned char *cmd,
 	scmd = blk_mq_rq_to_pdu(req);
 	scmd->cmd_len = COMMAND_SIZE(cmd[0]);
 	memcpy(scmd->cmnd, cmd, scmd->cmd_len);
-	rq->retries = retries;
+	scmd->allowed = retries;
 	req->timeout = timeout;
 	req->cmd_flags |= flags;
 	req->rq_flags |= rq_flags | RQF_QUIET;
@@ -1190,7 +1187,6 @@ static blk_status_t scsi_setup_scsi_cmnd(struct scsi_device *sdev,
 	}
 
 	cmd->transfersize = blk_rq_bytes(req);
-	cmd->allowed = scsi_req(req)->retries;
 	return BLK_STS_OK;
 }
 
diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c
index 4ee578b181da5..12bff64dade64 100644
--- a/drivers/scsi/scsi_transport_sas.c
+++ b/drivers/scsi/scsi_transport_sas.c
@@ -34,7 +34,6 @@
 
 #include <scsi/scsi.h>
 #include <scsi/scsi_cmnd.h>
-#include <scsi/scsi_request.h>
 #include <scsi/scsi_device.h>
 #include <scsi/scsi_host.h>
 #include <scsi/scsi_transport.h>
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 26a753521cb29..6a1c3ffaf32a0 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -1725,7 +1725,6 @@ sg_start_req(Sg_request *srp, unsigned char *cmd)
 {
 	int res;
 	struct request *rq;
-	struct scsi_request *req;
 	Sg_fd *sfp = srp->parentfp;
 	sg_io_hdr_t *hp = &srp->header;
 	int dxfer_len = (int) hp->dxfer_len;
@@ -1758,7 +1757,6 @@ sg_start_req(Sg_request *srp, unsigned char *cmd)
 	if (IS_ERR(rq))
 		return PTR_ERR(rq);
 	scmd = blk_mq_rq_to_pdu(rq);
-	req = scsi_req(rq);
 
 	if (hp->cmd_len > sizeof(scmd->cmnd)) {
 		blk_mq_free_request(rq);
@@ -1770,7 +1768,7 @@ sg_start_req(Sg_request *srp, unsigned char *cmd)
 
 	srp->rq = rq;
 	rq->end_io_data = srp;
-	req->retries = SG_DEFAULT_RETRIES;
+	scmd->allowed = SG_DEFAULT_RETRIES;
 
 	if ((dxfer_len <= 0) || (dxfer_dir == SG_DXFER_NONE))
 		return 0;
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 494d00b05f53e..aaa54ad5f0352 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -965,7 +965,6 @@ static int sr_read_cdda_bpc(struct cdrom_device_info *cdi, void __user *ubuf,
 {
 	struct gendisk *disk = cdi->disk;
 	u32 len = nr * CD_FRAMESIZE_RAW;
-	struct scsi_request *req;
 	struct scsi_cmnd *scmd;
 	struct request *rq;
 	struct bio *bio;
@@ -974,7 +973,6 @@ static int sr_read_cdda_bpc(struct cdrom_device_info *cdi, void __user *ubuf,
 	rq = scsi_alloc_request(disk->queue, REQ_OP_DRV_IN, 0);
 	if (IS_ERR(rq))
 		return PTR_ERR(rq);
-	req = scsi_req(rq);
 	scmd = blk_mq_rq_to_pdu(rq);
 
 	ret = blk_rq_map_user(disk->queue, rq, NULL, ubuf, len, GFP_KERNEL);
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index c8533ca225bc0..6d4213e2e49ae 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -539,7 +539,6 @@ static int st_scsi_execute(struct st_request *SRpnt, const unsigned char *cmd,
 			   int timeout, int retries)
 {
 	struct request *req;
-	struct scsi_request *rq;
 	struct rq_map_data *mdata = &SRpnt->stp->buffer->map_data;
 	int err = 0;
 	struct scsi_tape *STp = SRpnt->stp;
@@ -551,7 +550,6 @@ static int st_scsi_execute(struct st_request *SRpnt, const unsigned char *cmd,
 	if (IS_ERR(req))
 		return PTR_ERR(req);
 	scmd = blk_mq_rq_to_pdu(req);
-	rq = scsi_req(req);
 	req->rq_flags |= RQF_QUIET;
 
 	mdata->null_mapped = 1;
@@ -580,7 +578,7 @@ static int st_scsi_execute(struct st_request *SRpnt, const unsigned char *cmd,
 	scmd->cmd_len = COMMAND_SIZE(cmd[0]);
 	memcpy(scmd->cmnd, cmd, scmd->cmd_len);
 	req->timeout = timeout;
-	rq->retries = retries;
+	scmd->allowed = retries;
 	req->end_io_data = SRpnt;
 
 	blk_execute_rq_nowait(req, true, st_scsi_execute_end);
diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c
index 5b23a0ff905ea..d18d75d0d750c 100644
--- a/drivers/target/target_core_pscsi.c
+++ b/drivers/target/target_core_pscsi.c
@@ -993,7 +993,7 @@ pscsi_execute_cmd(struct se_cmd *cmd)
 		req->timeout = PS_TIMEOUT_DISK;
 	else
 		req->timeout = PS_TIMEOUT_OTHER;
-	scsi_req(req)->retries = PS_RETRY;
+	scmd->allowed = PS_RETRY;
 
 	cmd->priv = scmd->cmnd;
 
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 5ff0a6e8460c3..76c5eaeeb3b54 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -10,7 +10,6 @@
 #include <linux/timer.h>
 #include <linux/scatterlist.h>
 #include <scsi/scsi_device.h>
-#include <scsi/scsi_request.h>
 
 struct Scsi_Host;
 struct scsi_driver;
@@ -68,7 +67,6 @@ enum scsi_cmnd_submitter {
 } __packed;
 
 struct scsi_cmnd {
-	struct scsi_request req;
 	struct scsi_device *device;
 	struct list_head eh_entry; /* entry for the host eh_abort_list/eh_cmd_q */
 	struct delayed_work abort_work;
diff --git a/include/scsi/scsi_request.h b/include/scsi/scsi_request.h
deleted file mode 100644
index 929c7bd5c72fe..0000000000000
--- a/include/scsi/scsi_request.h
+++ /dev/null
@@ -1,16 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef _SCSI_SCSI_REQUEST_H
-#define _SCSI_SCSI_REQUEST_H
-
-#include <linux/blk-mq.h>
-
-struct scsi_request {
-	int		retries;
-};
-
-static inline struct scsi_request *scsi_req(struct request *rq)
-{
-	return blk_mq_rq_to_pdu(rq);
-}
-
-#endif /* _SCSI_SCSI_REQUEST_H */
-- 
2.30.2

Re: [PATCH 8/8] scsi: remove struct scsi_request

[Bart Van Assche]

On 2/22/22 06:04, Christoph Hellwig wrote:
> Let submitters initialized the scmd->allowed field directly instead of
                  ^^^^^^^^^^^
                  initialize?
> indirecting through struct scsi_request and remove the now superflous
> structure.

There are two changes in this patch:
- Initialize scmd->allowed directly instead of indirectly.
- Remove the include/scsi/scsi_request.h header.

You may want to split this patch into two patches - one patch per 
change. Anyway:

Reviewed-by: Bart Van Assche <bvanassche@acm.org>