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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

FWIW:

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

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




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux