diff mbox series

[v2,2/7] Change scsi_cmnd.prot_sdb from a pointer into a regular member

Message ID 20190123191013.119684-3-bvanassche@acm.org (mailing list archive)
State Not Applicable
Headers show
Series Fix handling of bidi commands | expand

Commit Message

Bart Van Assche Jan. 23, 2019, 7:10 p.m. UTC
This patch slightly increases the size of struct scsi_cmnd if data
protection is disabled, decreases the size of the data appended at
the end of struct scsi_cmnd if data protection is enabled and
simplifies the SCSI core.

Cc: Douglas Gilbert <dgilbert@interlog.com>
Cc: Hannes Reinecke <hare@suse.com>
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Bart Van Assche <bvanassche@acm.org>
---
 drivers/scsi/scsi_lib.c  | 33 ++++++++++++---------------------
 include/scsi/scsi_cmnd.h |  8 ++++----
 2 files changed, 16 insertions(+), 25 deletions(-)

Comments

Christoph Hellwig Jan. 29, 2019, 8:35 a.m. UTC | #1
On Wed, Jan 23, 2019 at 11:10:08AM -0800, Bart Van Assche wrote:
> This patch slightly increases the size of struct scsi_cmnd if data
> protection is disabled, decreases the size of the data appended at
> the end of struct scsi_cmnd if data protection is enabled and
> simplifies the SCSI core.

PI enabled devices are the absolutely minority of our setups.

I'm not against simplifying it per se, but this just doesn't seem
worth it.
diff mbox series

Patch

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 94842b104bcc..6bfbe50ef38e 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -566,7 +566,7 @@  static void scsi_mq_free_sgtables(struct scsi_cmnd *cmd)
 			sg_free_table_chained(&sdb->table, true);
 	}
 	if (scsi_prot_sg_count(cmd))
-		sg_free_table_chained(&cmd->prot_sdb->table, true);
+		sg_free_table_chained(&cmd->prot_sdb.table, true);
 }
 
 static void scsi_mq_uninit_cmd(struct scsi_cmnd *cmd)
@@ -1065,10 +1065,10 @@  blk_status_t scsi_init_io(struct scsi_cmnd *cmd)
 	}
 
 	if (blk_integrity_rq(rq)) {
-		struct scsi_data_buffer *prot_sdb = cmd->prot_sdb;
+		struct scsi_data_buffer *prot_sdb = &cmd->prot_sdb;
 		int ivecs, count;
 
-		if (WARN_ON_ONCE(!prot_sdb)) {
+		if (WARN_ON_ONCE(!scsi_host_get_prot(cmd->device->host))) {
 			/*
 			 * This can happen if someone (e.g. multipath)
 			 * queues a command to a device on an adapter
@@ -1091,8 +1091,7 @@  blk_status_t scsi_init_io(struct scsi_cmnd *cmd)
 		BUG_ON(count > ivecs);
 		BUG_ON(count > queue_max_integrity_segments(rq->q));
 
-		cmd->prot_sdb = prot_sdb;
-		cmd->prot_sdb->table.nents = count;
+		prot_sdb->table.nents = count;
 	}
 
 	return BLK_STS_OK;
@@ -1156,7 +1155,6 @@  void scsi_del_cmd_from_list(struct scsi_cmnd *cmd)
 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 = blk_mq_rq_from_pdu(cmd);
 	unsigned int flags = cmd->flags & SCMD_PRESERVED_FLAGS;
 	unsigned long jiffies_at_alloc;
@@ -1175,7 +1173,6 @@  void scsi_init_command(struct scsi_device *dev, struct scsi_cmnd *cmd)
 
 	cmd->device = dev;
 	cmd->sense_buffer = buf;
-	cmd->prot_sdb = prot;
 	cmd->flags = flags;
 	INIT_DELAYED_WORK(&cmd->abort_work, scmd_eh_abort_handler);
 	cmd->jiffies_at_alloc = jiffies_at_alloc;
@@ -1617,12 +1614,13 @@  static blk_status_t scsi_mq_prep_fn(struct request *req)
 	sg = (void *)cmd + sizeof(struct scsi_cmnd) + shost->hostt->cmd_size;
 	cmd->sdb.table.sgl = sg;
 
-	if (scsi_host_get_prot(shost)) {
-		memset(cmd->prot_sdb, 0, sizeof(struct scsi_data_buffer));
-
-		cmd->prot_sdb->table.sgl =
-			(struct scatterlist *)(cmd->prot_sdb + 1);
-	}
+	/*
+	 * Always initialize cmd->prot_sdb.nents such that
+	 * scsi_prot_sg_count() does not have to call scsi_host_get_prot().
+	 */
+	memset(&cmd->prot_sdb, 0, sizeof(struct scsi_data_buffer));
+	if (scsi_host_get_prot(shost))
+		cmd->prot_sdb.table.sgl = (void *)&sg + scsi_mq_sgl_size(shost);
 
 	if (blk_bidi_rq(req)) {
 		struct request *next_rq = req->next_rq;
@@ -1781,7 +1779,6 @@  static int scsi_mq_init_request(struct blk_mq_tag_set *set, struct request *rq,
 	struct Scsi_Host *shost = set->driver_data;
 	const bool unchecked_isa_dma = shost->unchecked_isa_dma;
 	struct scsi_cmnd *cmd = blk_mq_rq_to_pdu(rq);
-	struct scatterlist *sg;
 
 	if (unchecked_isa_dma)
 		cmd->flags |= SCMD_UNCHECKED_ISA_DMA;
@@ -1791,12 +1788,6 @@  static int scsi_mq_init_request(struct blk_mq_tag_set *set, struct request *rq,
 		return -ENOMEM;
 	cmd->req.sense = cmd->sense_buffer;
 
-	if (scsi_host_get_prot(shost)) {
-		sg = (void *)cmd + sizeof(struct scsi_cmnd) +
-			shost->hostt->cmd_size;
-		cmd->prot_sdb = (void *)sg + scsi_mq_sgl_size(shost);
-	}
-
 	return 0;
 }
 
@@ -1893,7 +1884,7 @@  int scsi_mq_setup_tags(struct Scsi_Host *shost)
 	sgl_size = scsi_mq_sgl_size(shost);
 	cmd_size = sizeof(struct scsi_cmnd) + shost->hostt->cmd_size + sgl_size;
 	if (scsi_host_get_prot(shost))
-		cmd_size += sizeof(struct scsi_data_buffer) + sgl_size;
+		cmd_size += sgl_size;
 
 	memset(&shost->tag_set, 0, sizeof(shost->tag_set));
 	shost->tag_set.ops = &scsi_mq_ops;
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index d85e6befa26b..0406c0fbee3e 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -108,7 +108,7 @@  struct scsi_cmnd {
 
 	/* These elements define the operation we ultimately want to perform */
 	struct scsi_data_buffer sdb;
-	struct scsi_data_buffer *prot_sdb;
+	struct scsi_data_buffer prot_sdb;
 
 	unsigned underflow;	/* Return error if less than
 				   this amount is transferred */
@@ -318,17 +318,17 @@  static inline unsigned int scsi_prot_interval(struct scsi_cmnd *scmd)
 
 static inline unsigned scsi_prot_sg_count(struct scsi_cmnd *cmd)
 {
-	return cmd->prot_sdb ? cmd->prot_sdb->table.nents : 0;
+	return cmd->prot_sdb.table.nents;
 }
 
 static inline struct scatterlist *scsi_prot_sglist(struct scsi_cmnd *cmd)
 {
-	return cmd->prot_sdb ? cmd->prot_sdb->table.sgl : NULL;
+	return cmd->prot_sdb.table.sgl;
 }
 
 static inline struct scsi_data_buffer *scsi_prot(struct scsi_cmnd *cmd)
 {
-	return cmd->prot_sdb;
+	return &cmd->prot_sdb;
 }
 
 #define scsi_for_each_prot_sg(cmd, sg, nseg, __i)		\