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

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

 



On 24/02/2022 17:55, Christoph Hellwig wrote:
Now that each scsi_request is backed by a scsi_cmnd, there is no
need to indirect the CDB storage.  Change all submitters of SCSI
passthrough requests 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@xxxxxx>
Reviewed-by: Bart Van Assche <bvanassche@xxxxxxx>

I had just finished going through the v1 of this patch and this seems identical, so, FWIW:

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

Just a minor comment below (feel free to ignore).

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

You might be able to move this check earlier (before the request allocation), but no big deal.




[Index of Archives]     [Linux SCSI]     [Kernel Newbies]     [Linux SCSI Target Infrastructure]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Device Mapper]

  Powered by Linux