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
