diff mbox

[v2,09/12] Make scsi_mq_prep_fn() call scsi_init_command()

Message ID 20170601232711.29062-10-bart.vanassche@sandisk.com (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Bart Van Assche June 1, 2017, 11:27 p.m. UTC
This patch reduces code duplication. The only functional change in
this patch is that it causes scsi_mq_prep_fn() clear driver-private
command data, just like the already upstream commit 1bad6c4a57ef
("scsi: zero per-cmd private driver data for each MQ I/O").

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Johannes Thumshirn <jthumshirn@suse.de>
---
 drivers/scsi/scsi_error.c |  2 +-
 drivers/scsi/scsi_lib.c   | 28 +++++++---------------------
 drivers/scsi/scsi_priv.h  |  4 +++-
 3 files changed, 11 insertions(+), 23 deletions(-)

Comments

Christoph Hellwig June 2, 2017, 7:22 a.m. UTC | #1
I like this idea, but..

> -void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd)
> +void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd,
> +		       struct scsi_data_buffer *prot_sdb)
>  {
>  	void *buf = cmd->sense_buffer;
> -	void *prot = cmd->prot_sdb;
>  	unsigned int unchecked_isa_dma = cmd->flags & SCMD_UNCHECKED_ISA_DMA;
>  
>  	/* zero out the cmd, except for the embedded scsi_request */
> @@ -1164,7 +1164,7 @@ void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd)
>  
>  	cmd->device = dev;
>  	cmd->sense_buffer = buf;
> -	cmd->prot_sdb = prot;
> +	cmd->prot_sdb = prot_sdb;

What would be the problem of always preserving the original prot_sdb
value instead of passing it by argument?  You;d just need to initialize
it in scsi_init_request when the command is allocated.
Bart Van Assche June 2, 2017, 5:49 p.m. UTC | #2
On Fri, 2017-06-02 at 09:22 +0200, Christoph Hellwig wrote:
> I like this idea, but..
> 
> > -void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd)
> > +void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd,
> > +		       struct scsi_data_buffer *prot_sdb)
> >  {
> >  	void *buf = cmd->sense_buffer;
> > -	void *prot = cmd->prot_sdb;
> >  	unsigned int unchecked_isa_dma = cmd->flags & SCMD_UNCHECKED_ISA_DMA;
> >  
> >  	/* zero out the cmd, except for the embedded scsi_request */
> > @@ -1164,7 +1164,7 @@ void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd)
> >  
> >  	cmd->device = dev;
> >  	cmd->sense_buffer = buf;
> > -	cmd->prot_sdb = prot;
> > +	cmd->prot_sdb = prot_sdb;
> 
> What would be the problem of always preserving the original prot_sdb
> value instead of passing it by argument?  You;d just need to initialize
> it in scsi_init_request when the command is allocated.

Hello Christoph,

That sounds like a good idea to me. I will make that change.

Bart.
diff mbox

Patch

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index ac3196420435..2e73ef6c1857 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -2283,7 +2283,7 @@  scsi_ioctl_reset(struct scsi_device *dev, int __user *arg)
 	blk_rq_init(NULL, rq);
 
 	scmd = (struct scsi_cmnd *)(rq + 1);
-	scsi_init_command(dev, scmd);
+	scsi_init_command(dev, scmd, NULL);
 	scmd->request = rq;
 	scmd->cmnd = scsi_req(rq)->cmd;
 
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 6b4fb48033fb..4041d4c70845 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1152,10 +1152,10 @@  void scsi_del_cmd_from_list(struct scsi_cmnd *cmd)
 	}
 }
 
-void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd)
+void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd,
+		       struct scsi_data_buffer *prot_sdb)
 {
 	void *buf = cmd->sense_buffer;
-	void *prot = cmd->prot_sdb;
 	unsigned int unchecked_isa_dma = cmd->flags & SCMD_UNCHECKED_ISA_DMA;
 
 	/* zero out the cmd, except for the embedded scsi_request */
@@ -1164,7 +1164,7 @@  void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd)
 
 	cmd->device = dev;
 	cmd->sense_buffer = buf;
-	cmd->prot_sdb = prot;
+	cmd->prot_sdb = prot_sdb;
 	cmd->flags = unchecked_isa_dma;
 	INIT_DELAYED_WORK(&cmd->abort_work, scmd_eh_abort_handler);
 	cmd->jiffies_at_alloc = jiffies;
@@ -1342,7 +1342,7 @@  static int scsi_prep_fn(struct request_queue *q, struct request *req)
 			goto out;
 		}
 
-		scsi_init_command(sdev, cmd);
+		scsi_init_command(sdev, cmd, cmd->prot_sdb);
 		req->special = cmd;
 	}
 
@@ -1870,36 +1870,22 @@  static int scsi_mq_prep_fn(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;
-	unsigned char *sense_buf = cmd->sense_buffer;
-	unsigned int unchecked_isa_dma = cmd->flags & SCMD_UNCHECKED_ISA_DMA;
 	struct scatterlist *sg;
 
-	/* zero out the cmd, except for the embedded scsi_request */
-	memset((char *)cmd + sizeof(cmd->req), 0,
-		sizeof(*cmd) - sizeof(cmd->req));
+	sg = (void *)cmd + sizeof(struct scsi_cmnd) + shost->hostt->cmd_size;
+	scsi_init_command(sdev, cmd, scsi_host_get_prot(shost) ?
+			  (void *)sg + scsi_mq_sgl_size(shost) : NULL);
 
 	req->special = cmd;
 
 	cmd->request = req;
-	cmd->device = sdev;
-	cmd->sense_buffer = sense_buf;
-	cmd->flags = unchecked_isa_dma;
 
 	cmd->tag = req->tag;
-
 	cmd->prot_op = SCSI_PROT_NORMAL;
 
-	INIT_LIST_HEAD(&cmd->list);
-	INIT_DELAYED_WORK(&cmd->abort_work, scmd_eh_abort_handler);
-	cmd->jiffies_at_alloc = jiffies;
-
-	scsi_add_cmd_to_list(cmd);
-
-	sg = (void *)cmd + sizeof(struct scsi_cmnd) + shost->hostt->cmd_size;
 	cmd->sdb.table.sgl = sg;
 
 	if (scsi_host_get_prot(shost)) {
-		cmd->prot_sdb = (void *)sg + scsi_mq_sgl_size(shost);
 		memset(cmd->prot_sdb, 0, sizeof(struct scsi_data_buffer));
 
 		cmd->prot_sdb->table.sgl =
diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index c11c1f9c912c..c43a138423d7 100644
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -8,6 +8,7 @@ 
 struct request_queue;
 struct request;
 struct scsi_cmnd;
+struct scsi_data_buffer;
 struct scsi_device;
 struct scsi_target;
 struct scsi_host_template;
@@ -30,7 +31,8 @@  extern void scsi_exit_hosts(void);
 /* scsi.c */
 extern bool scsi_use_blk_mq;
 int scsi_init_sense_cache(struct Scsi_Host *shost);
-void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd);
+void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd,
+		       struct scsi_data_buffer *prot_sdb);
 #ifdef CONFIG_SCSI_LOGGING
 void scsi_log_send(struct scsi_cmnd *cmd);
 void scsi_log_completion(struct scsi_cmnd *cmd, int disposition);