public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
* RFC: struct request cleanups
@ 2015-04-17 20:37 Christoph Hellwig
  2015-04-17 20:37 ` [PATCH 1/7] block: rename REQ_TYPE_SPECIAL to REQ_TYPE_DRV_PRIV Christoph Hellwig
                   ` (8 more replies)
  0 siblings, 9 replies; 12+ messages in thread
From: Christoph Hellwig @ 2015-04-17 20:37 UTC (permalink / raw)
  To: axboe, linux-nvme; +Cc: linux-scsi

The first 5 patches move the magic IDE request types into the old IDE
driver to keep the core block code clean of them.  Those are basically
ready to merge, just like the 6th one which is a cleanup on it's own.

The real RFC is the last one which allocates the block_pc specific
data separately in the callers instead of bloating every struct
request with it.  I always hated what we did, but with the upcoming
split of nvme into transports and command sets we'll need a NVME
equivalent of BLOCK_PC, and as NVMe was designed by crackmonkeys
dreaming of an ATA controller the "command block" for NVME is even
bigger than what we have to deal with in SCSI.

Note that the old IDE driver doesn't compile with the last patch
yet as there are major nightmares to sort out, and BLOCK_PC passthrough
with dm-multipath doesn't work yet either.  If I get some general
concensus on the approach I'll fix those of course.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/7] block: rename REQ_TYPE_SPECIAL to REQ_TYPE_DRV_PRIV
  2015-04-17 20:37 RFC: struct request cleanups Christoph Hellwig
@ 2015-04-17 20:37 ` Christoph Hellwig
  2015-04-17 20:37 ` [PATCH 2/7] block: move REQ_TYPE_ATA_TASKFILE and REQ_TYPE_ATA_PC to ide.h Christoph Hellwig
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2015-04-17 20:37 UTC (permalink / raw)
  To: axboe, linux-nvme; +Cc: linux-scsi

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/nbd.c        | 2 +-
 drivers/block/paride/pd.c  | 4 ++--
 drivers/block/sx8.c        | 4 ++--
 drivers/block/virtio_blk.c | 6 +++---
 drivers/ide/ide-atapi.c    | 4 ++--
 drivers/ide/ide-cd.c       | 2 +-
 drivers/ide/ide-cd_ioctl.c | 2 +-
 drivers/ide/ide-devsets.c  | 2 +-
 drivers/ide/ide-eh.c       | 2 +-
 drivers/ide/ide-floppy.c   | 6 +++---
 drivers/ide/ide-io.c       | 4 ++--
 drivers/ide/ide-ioctls.c   | 2 +-
 drivers/ide/ide-park.c     | 4 ++--
 drivers/ide/ide-tape.c     | 4 ++--
 include/linux/blkdev.h     | 4 ++--
 15 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 39e5f7f..9cf52ac 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -592,7 +592,7 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
 		fsync_bdev(bdev);
 		mutex_lock(&nbd->tx_lock);
 		blk_rq_init(NULL, &sreq);
-		sreq.cmd_type = REQ_TYPE_SPECIAL;
+		sreq.cmd_type = REQ_TYPE_DRV_PRIV;
 		nbd_cmd(&sreq) = NBD_CMD_DISC;
 
 		/* Check again after getting mutex back.  */
diff --git a/drivers/block/paride/pd.c b/drivers/block/paride/pd.c
index d48715b..dbb4da1 100644
--- a/drivers/block/paride/pd.c
+++ b/drivers/block/paride/pd.c
@@ -442,7 +442,7 @@ static char *pd_buf;		/* buffer for request in progress */
 
 static enum action do_pd_io_start(void)
 {
-	if (pd_req->cmd_type == REQ_TYPE_SPECIAL) {
+	if (pd_req->cmd_type == REQ_TYPE_DRV_PRIV) {
 		phase = pd_special;
 		return pd_special();
 	}
@@ -725,7 +725,7 @@ static int pd_special_command(struct pd_unit *disk,
 	if (IS_ERR(rq))
 		return PTR_ERR(rq);
 
-	rq->cmd_type = REQ_TYPE_SPECIAL;
+	rq->cmd_type = REQ_TYPE_DRV_PRIV;
 	rq->special = func;
 
 	err = blk_execute_rq(disk->gd->queue, disk->gd, rq, 0);
diff --git a/drivers/block/sx8.c b/drivers/block/sx8.c
index 5d55285..59c91d4 100644
--- a/drivers/block/sx8.c
+++ b/drivers/block/sx8.c
@@ -620,7 +620,7 @@ static int carm_array_info (struct carm_host *host, unsigned int array_idx)
 	spin_unlock_irq(&host->lock);
 
 	DPRINTK("blk_execute_rq_nowait, tag == %u\n", idx);
-	crq->rq->cmd_type = REQ_TYPE_SPECIAL;
+	crq->rq->cmd_type = REQ_TYPE_DRV_PRIV;
 	crq->rq->special = crq;
 	blk_execute_rq_nowait(host->oob_q, NULL, crq->rq, true, NULL);
 
@@ -661,7 +661,7 @@ static int carm_send_special (struct carm_host *host, carm_sspc_t func)
 	crq->msg_bucket = (u32) rc;
 
 	DPRINTK("blk_execute_rq_nowait, tag == %u\n", idx);
-	crq->rq->cmd_type = REQ_TYPE_SPECIAL;
+	crq->rq->cmd_type = REQ_TYPE_DRV_PRIV;
 	crq->rq->special = crq;
 	blk_execute_rq_nowait(host->oob_q, NULL, crq->rq, true, NULL);
 
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index 5ea2f0b..d4d05f0 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -124,7 +124,7 @@ static inline void virtblk_request_done(struct request *req)
 		req->resid_len = virtio32_to_cpu(vblk->vdev, vbr->in_hdr.residual);
 		req->sense_len = virtio32_to_cpu(vblk->vdev, vbr->in_hdr.sense_len);
 		req->errors = virtio32_to_cpu(vblk->vdev, vbr->in_hdr.errors);
-	} else if (req->cmd_type == REQ_TYPE_SPECIAL) {
+	} else if (req->cmd_type == REQ_TYPE_DRV_PRIV) {
 		req->errors = (error != 0);
 	}
 
@@ -188,7 +188,7 @@ static int virtio_queue_rq(struct blk_mq_hw_ctx *hctx,
 			vbr->out_hdr.sector = 0;
 			vbr->out_hdr.ioprio = cpu_to_virtio32(vblk->vdev, req_get_ioprio(vbr->req));
 			break;
-		case REQ_TYPE_SPECIAL:
+		case REQ_TYPE_DRV_PRIV:
 			vbr->out_hdr.type = cpu_to_virtio32(vblk->vdev, VIRTIO_BLK_T_GET_ID);
 			vbr->out_hdr.sector = 0;
 			vbr->out_hdr.ioprio = cpu_to_virtio32(vblk->vdev, req_get_ioprio(vbr->req));
@@ -251,7 +251,7 @@ static int virtblk_get_id(struct gendisk *disk, char *id_str)
 		return PTR_ERR(req);
 	}
 
-	req->cmd_type = REQ_TYPE_SPECIAL;
+	req->cmd_type = REQ_TYPE_DRV_PRIV;
 	err = blk_execute_rq(vblk->disk->queue, vblk->disk, req, false);
 	blk_put_request(req);
 
diff --git a/drivers/ide/ide-atapi.c b/drivers/ide/ide-atapi.c
index fac3d9d..b367300 100644
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -93,7 +93,7 @@ int ide_queue_pc_tail(ide_drive_t *drive, struct gendisk *disk,
 	int error;
 
 	rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
-	rq->cmd_type = REQ_TYPE_SPECIAL;
+	rq->cmd_type = REQ_TYPE_DRV_PRIV;
 	rq->special = (char *)pc;
 
 	if (buf && bufflen) {
@@ -477,7 +477,7 @@ static ide_startstop_t ide_pc_intr(ide_drive_t *drive)
 		if (uptodate == 0)
 			drive->failed_pc = NULL;
 
-		if (rq->cmd_type == REQ_TYPE_SPECIAL) {
+		if (rq->cmd_type == REQ_TYPE_DRV_PRIV) {
 			rq->errors = 0;
 			error = 0;
 		} else {
diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index 0b510ba..9a32603 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -799,7 +799,7 @@ static ide_startstop_t ide_cd_do_request(ide_drive_t *drive, struct request *rq,
 
 		cdrom_do_block_pc(drive, rq);
 		break;
-	case REQ_TYPE_SPECIAL:
+	case REQ_TYPE_DRV_PRIV:
 		/* right now this can only be a reset... */
 		uptodate = 1;
 		goto out_end;
diff --git a/drivers/ide/ide-cd_ioctl.c b/drivers/ide/ide-cd_ioctl.c
index 02caa7d..066e390 100644
--- a/drivers/ide/ide-cd_ioctl.c
+++ b/drivers/ide/ide-cd_ioctl.c
@@ -304,7 +304,7 @@ int ide_cdrom_reset(struct cdrom_device_info *cdi)
 	int ret;
 
 	rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
-	rq->cmd_type = REQ_TYPE_SPECIAL;
+	rq->cmd_type = REQ_TYPE_DRV_PRIV;
 	rq->cmd_flags = REQ_QUIET;
 	ret = blk_execute_rq(drive->queue, cd->disk, rq, 0);
 	blk_put_request(rq);
diff --git a/drivers/ide/ide-devsets.c b/drivers/ide/ide-devsets.c
index 9e98122..b05a74d 100644
--- a/drivers/ide/ide-devsets.c
+++ b/drivers/ide/ide-devsets.c
@@ -166,7 +166,7 @@ int ide_devset_execute(ide_drive_t *drive, const struct ide_devset *setting,
 		return setting->set(drive, arg);
 
 	rq = blk_get_request(q, READ, __GFP_WAIT);
-	rq->cmd_type = REQ_TYPE_SPECIAL;
+	rq->cmd_type = REQ_TYPE_DRV_PRIV;
 	rq->cmd_len = 5;
 	rq->cmd[0] = REQ_DEVSET_EXEC;
 	*(int *)&rq->cmd[1] = arg;
diff --git a/drivers/ide/ide-eh.c b/drivers/ide/ide-eh.c
index 3297066..19d809c 100644
--- a/drivers/ide/ide-eh.c
+++ b/drivers/ide/ide-eh.c
@@ -147,7 +147,7 @@ static inline void ide_complete_drive_reset(ide_drive_t *drive, int err)
 {
 	struct request *rq = drive->hwif->rq;
 
-	if (rq && rq->cmd_type == REQ_TYPE_SPECIAL &&
+	if (rq && rq->cmd_type == REQ_TYPE_DRV_PRIV &&
 	    rq->cmd[0] == REQ_DRIVE_RESET) {
 		if (err <= 0 && rq->errors == 0)
 			rq->errors = -EIO;
diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 8c6363c..3dbfa5b 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -97,7 +97,7 @@ static int ide_floppy_callback(ide_drive_t *drive, int dsc)
 			       "Aborting request!\n");
 	}
 
-	if (rq->cmd_type == REQ_TYPE_SPECIAL)
+	if (rq->cmd_type == REQ_TYPE_DRV_PRIV)
 		rq->errors = uptodate ? 0 : IDE_DRV_ERROR_GENERAL;
 
 	return uptodate;
@@ -246,7 +246,7 @@ static ide_startstop_t ide_floppy_do_request(ide_drive_t *drive,
 		} else
 			printk(KERN_ERR PFX "%s: I/O error\n", drive->name);
 
-		if (rq->cmd_type == REQ_TYPE_SPECIAL) {
+		if (rq->cmd_type == REQ_TYPE_DRV_PRIV) {
 			rq->errors = 0;
 			ide_complete_rq(drive, 0, blk_rq_bytes(rq));
 			return ide_stopped;
@@ -265,7 +265,7 @@ static ide_startstop_t ide_floppy_do_request(ide_drive_t *drive,
 		pc = &floppy->queued_pc;
 		idefloppy_create_rw_cmd(drive, pc, rq, (unsigned long)block);
 		break;
-	case REQ_TYPE_SPECIAL:
+	case REQ_TYPE_DRV_PRIV:
 	case REQ_TYPE_SENSE:
 		pc = (struct ide_atapi_pc *)rq->special;
 		break;
diff --git a/drivers/ide/ide-io.c b/drivers/ide/ide-io.c
index 177db6d..8e55abd 100644
--- a/drivers/ide/ide-io.c
+++ b/drivers/ide/ide-io.c
@@ -135,7 +135,7 @@ EXPORT_SYMBOL(ide_complete_rq);
 
 void ide_kill_rq(ide_drive_t *drive, struct request *rq)
 {
-	u8 drv_req = (rq->cmd_type == REQ_TYPE_SPECIAL) && rq->rq_disk;
+	u8 drv_req = (rq->cmd_type == REQ_TYPE_DRV_PRIV) && rq->rq_disk;
 	u8 media = drive->media;
 
 	drive->failed_pc = NULL;
@@ -353,7 +353,7 @@ static ide_startstop_t start_request (ide_drive_t *drive, struct request *rq)
 			    pm->pm_step == IDE_PM_COMPLETED)
 				ide_complete_pm_rq(drive, rq);
 			return startstop;
-		} else if (!rq->rq_disk && rq->cmd_type == REQ_TYPE_SPECIAL)
+		} else if (!rq->rq_disk && rq->cmd_type == REQ_TYPE_DRV_PRIV)
 			/*
 			 * TODO: Once all ULDs have been modified to
 			 * check for specific op codes rather than
diff --git a/drivers/ide/ide-ioctls.c b/drivers/ide/ide-ioctls.c
index 6233fa2c..aa2e9b7 100644
--- a/drivers/ide/ide-ioctls.c
+++ b/drivers/ide/ide-ioctls.c
@@ -222,7 +222,7 @@ static int generic_drive_reset(ide_drive_t *drive)
 	int ret = 0;
 
 	rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
-	rq->cmd_type = REQ_TYPE_SPECIAL;
+	rq->cmd_type = REQ_TYPE_DRV_PRIV;
 	rq->cmd_len = 1;
 	rq->cmd[0] = REQ_DRIVE_RESET;
 	if (blk_execute_rq(drive->queue, NULL, rq, 1))
diff --git a/drivers/ide/ide-park.c b/drivers/ide/ide-park.c
index ca95860..c808685 100644
--- a/drivers/ide/ide-park.c
+++ b/drivers/ide/ide-park.c
@@ -34,7 +34,7 @@ static void issue_park_cmd(ide_drive_t *drive, unsigned long timeout)
 	rq = blk_get_request(q, READ, __GFP_WAIT);
 	rq->cmd[0] = REQ_PARK_HEADS;
 	rq->cmd_len = 1;
-	rq->cmd_type = REQ_TYPE_SPECIAL;
+	rq->cmd_type = REQ_TYPE_DRV_PRIV;
 	rq->special = &timeout;
 	rc = blk_execute_rq(q, NULL, rq, 1);
 	blk_put_request(rq);
@@ -51,7 +51,7 @@ static void issue_park_cmd(ide_drive_t *drive, unsigned long timeout)
 
 	rq->cmd[0] = REQ_UNPARK_HEADS;
 	rq->cmd_len = 1;
-	rq->cmd_type = REQ_TYPE_SPECIAL;
+	rq->cmd_type = REQ_TYPE_DRV_PRIV;
 	elv_add_request(q, rq, ELEVATOR_INSERT_FRONT);
 
 out:
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 6eb738c..2a5d543 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -576,7 +576,7 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive,
 		      rq->cmd[0], (unsigned long long)blk_rq_pos(rq),
 		      blk_rq_sectors(rq));
 
-	BUG_ON(!(rq->cmd_type == REQ_TYPE_SPECIAL ||
+	BUG_ON(!(rq->cmd_type == REQ_TYPE_DRV_PRIV ||
 		 rq->cmd_type == REQ_TYPE_SENSE));
 
 	/* Retry a failed packet command */
@@ -853,7 +853,7 @@ static int idetape_queue_rw_tail(ide_drive_t *drive, int cmd, int size)
 	BUG_ON(size < 0 || size % tape->blk_size);
 
 	rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
-	rq->cmd_type = REQ_TYPE_SPECIAL;
+	rq->cmd_type = REQ_TYPE_DRV_PRIV;
 	rq->cmd[13] = cmd;
 	rq->rq_disk = tape->disk;
 	rq->__sector = tape->first_frame;
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 7f9a516..98c9027 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -79,10 +79,10 @@ enum rq_cmd_type_bits {
 	REQ_TYPE_PM_SUSPEND,		/* suspend request */
 	REQ_TYPE_PM_RESUME,		/* resume request */
 	REQ_TYPE_PM_SHUTDOWN,		/* shutdown request */
-	REQ_TYPE_SPECIAL,		/* driver defined type */
+	REQ_TYPE_DRV_PRIV,		/* driver defined type */
 	/*
 	 * for ATA/ATAPI devices. this really doesn't belong here, ide should
-	 * use REQ_TYPE_SPECIAL and use rq->cmd[0] with the range of driver
+	 * use REQ_TYPE_DRV_PRIV and use rq->cmd[0] with the range of driver
 	 * private REQ_LB opcodes to differentiate what type of request this is
 	 */
 	REQ_TYPE_ATA_TASKFILE,
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/7] block: move REQ_TYPE_ATA_TASKFILE and REQ_TYPE_ATA_PC to ide.h
  2015-04-17 20:37 RFC: struct request cleanups Christoph Hellwig
  2015-04-17 20:37 ` [PATCH 1/7] block: rename REQ_TYPE_SPECIAL to REQ_TYPE_DRV_PRIV Christoph Hellwig
@ 2015-04-17 20:37 ` Christoph Hellwig
  2015-04-17 20:37 ` [PATCH 3/7] block: move REQ_TYPE_SENSE to the ide driver Christoph Hellwig
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2015-04-17 20:37 UTC (permalink / raw)
  To: axboe, linux-nvme; +Cc: linux-scsi

These values are only used by the IDE driver, so move them into it
by allowing drivers to take cmd_type values after the first private
one.  Note that we have to turn cmd_type into a plain unsigned integer
so that gcc doesn't complain about mismatching enum types.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/linux/blkdev.h | 11 ++---------
 include/linux/ide.h    |  7 +++++++
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 98c9027..9cb4d80 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -79,14 +79,7 @@ enum rq_cmd_type_bits {
 	REQ_TYPE_PM_SUSPEND,		/* suspend request */
 	REQ_TYPE_PM_RESUME,		/* resume request */
 	REQ_TYPE_PM_SHUTDOWN,		/* shutdown request */
-	REQ_TYPE_DRV_PRIV,		/* driver defined type */
-	/*
-	 * for ATA/ATAPI devices. this really doesn't belong here, ide should
-	 * use REQ_TYPE_DRV_PRIV and use rq->cmd[0] with the range of driver
-	 * private REQ_LB opcodes to differentiate what type of request this is
-	 */
-	REQ_TYPE_ATA_TASKFILE,
-	REQ_TYPE_ATA_PC,
+	REQ_TYPE_DRV_PRIV,		/* driver defined types from here */
 };
 
 #define BLK_MAX_CDB	16
@@ -108,7 +101,7 @@ struct request {
 	struct blk_mq_ctx *mq_ctx;
 
 	u64 cmd_flags;
-	enum rq_cmd_type_bits cmd_type;
+	unsigned cmd_type;
 	unsigned long atomic_flags;
 
 	int cpu;
diff --git a/include/linux/ide.h b/include/linux/ide.h
index 93b5ca7..62ac399 100644
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -39,6 +39,12 @@
 
 struct device;
 
+/* IDE-specific values for req->cmd_type */
+enum ata_cmd_type_bits {
+	REQ_TYPE_ATA_TASKFILE = REQ_TYPE_DRV_PRIV + 1,
+	REQ_TYPE_ATA_PC,
+};
+
 /* Error codes returned in rq->errors to the higher part of the driver. */
 enum {
 	IDE_DRV_ERROR_GENERAL	= 101,
@@ -1551,4 +1557,5 @@ static inline void ide_set_drivedata(ide_drive_t *drive, void *data)
 #define ide_host_for_each_port(i, port, host) \
 	for ((i) = 0; ((port) = (host)->ports[i]) || (i) < MAX_HOST_PORTS; (i)++)
 
+
 #endif /* _IDE_H */
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/7] block: move REQ_TYPE_SENSE to the ide driver
  2015-04-17 20:37 RFC: struct request cleanups Christoph Hellwig
  2015-04-17 20:37 ` [PATCH 1/7] block: rename REQ_TYPE_SPECIAL to REQ_TYPE_DRV_PRIV Christoph Hellwig
  2015-04-17 20:37 ` [PATCH 2/7] block: move REQ_TYPE_ATA_TASKFILE and REQ_TYPE_ATA_PC to ide.h Christoph Hellwig
@ 2015-04-17 20:37 ` Christoph Hellwig
  2015-04-17 20:37 ` [PATCH 4/7] block: remove REQ_TYPE_PM_SHUTDOWN Christoph Hellwig
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2015-04-17 20:37 UTC (permalink / raw)
  To: axboe, linux-nvme; +Cc: linux-scsi

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/ide/ide-atapi.c  | 6 +++---
 drivers/ide/ide-cd.c     | 8 ++++----
 drivers/ide/ide-floppy.c | 2 +-
 drivers/ide/ide-tape.c   | 2 +-
 include/linux/blkdev.h   | 1 -
 include/linux/ide.h      | 1 +
 6 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/ide/ide-atapi.c b/drivers/ide/ide-atapi.c
index b367300..1362ad8 100644
--- a/drivers/ide/ide-atapi.c
+++ b/drivers/ide/ide-atapi.c
@@ -191,7 +191,7 @@ void ide_prep_sense(ide_drive_t *drive, struct request *rq)
 
 	BUG_ON(sense_len > sizeof(*sense));
 
-	if (rq->cmd_type == REQ_TYPE_SENSE || drive->sense_rq_armed)
+	if (rq->cmd_type == REQ_TYPE_ATA_SENSE || drive->sense_rq_armed)
 		return;
 
 	memset(sense, 0, sizeof(*sense));
@@ -210,7 +210,7 @@ void ide_prep_sense(ide_drive_t *drive, struct request *rq)
 	sense_rq->rq_disk = rq->rq_disk;
 	sense_rq->cmd[0] = GPCMD_REQUEST_SENSE;
 	sense_rq->cmd[4] = cmd_len;
-	sense_rq->cmd_type = REQ_TYPE_SENSE;
+	sense_rq->cmd_type = REQ_TYPE_ATA_SENSE;
 	sense_rq->cmd_flags |= REQ_PREEMPT;
 
 	if (drive->media == ide_tape)
@@ -310,7 +310,7 @@ int ide_cd_get_xferlen(struct request *rq)
 	switch (rq->cmd_type) {
 	case REQ_TYPE_FS:
 		return 32768;
-	case REQ_TYPE_SENSE:
+	case REQ_TYPE_ATA_SENSE:
 	case REQ_TYPE_BLOCK_PC:
 	case REQ_TYPE_ATA_PC:
 		return blk_rq_bytes(rq);
diff --git a/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c
index 9a32603..64a6b82 100644
--- a/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c
@@ -210,7 +210,7 @@ static void cdrom_analyze_sense_data(ide_drive_t *drive,
 static void ide_cd_complete_failed_rq(ide_drive_t *drive, struct request *rq)
 {
 	/*
-	 * For REQ_TYPE_SENSE, "rq->special" points to the original
+	 * For REQ_TYPE_ATA_SENSE, "rq->special" points to the original
 	 * failed request.  Also, the sense data should be read
 	 * directly from rq which might be different from the original
 	 * sense buffer if it got copied during mapping.
@@ -285,7 +285,7 @@ static int cdrom_decode_status(ide_drive_t *drive, u8 stat)
 				  "stat 0x%x",
 				  rq->cmd[0], rq->cmd_type, err, stat);
 
-	if (rq->cmd_type == REQ_TYPE_SENSE) {
+	if (rq->cmd_type == REQ_TYPE_ATA_SENSE) {
 		/*
 		 * We got an error trying to get sense info from the drive
 		 * (probably while trying to recover from a former error).
@@ -526,7 +526,7 @@ static ide_startstop_t cdrom_newpc_intr(ide_drive_t *drive)
 	ide_expiry_t *expiry = NULL;
 	int dma_error = 0, dma, thislen, uptodate = 0;
 	int write = (rq_data_dir(rq) == WRITE) ? 1 : 0, rc = 0;
-	int sense = (rq->cmd_type == REQ_TYPE_SENSE);
+	int sense = (rq->cmd_type == REQ_TYPE_ATA_SENSE);
 	unsigned int timeout;
 	u16 len;
 	u8 ireason, stat;
@@ -791,7 +791,7 @@ static ide_startstop_t ide_cd_do_request(ide_drive_t *drive, struct request *rq,
 		if (cdrom_start_rw(drive, rq) == ide_stopped)
 			goto out_end;
 		break;
-	case REQ_TYPE_SENSE:
+	case REQ_TYPE_ATA_SENSE:
 	case REQ_TYPE_BLOCK_PC:
 	case REQ_TYPE_ATA_PC:
 		if (!rq->timeout)
diff --git a/drivers/ide/ide-floppy.c b/drivers/ide/ide-floppy.c
index 3dbfa5b..2fb5350 100644
--- a/drivers/ide/ide-floppy.c
+++ b/drivers/ide/ide-floppy.c
@@ -266,7 +266,7 @@ static ide_startstop_t ide_floppy_do_request(ide_drive_t *drive,
 		idefloppy_create_rw_cmd(drive, pc, rq, (unsigned long)block);
 		break;
 	case REQ_TYPE_DRV_PRIV:
-	case REQ_TYPE_SENSE:
+	case REQ_TYPE_ATA_SENSE:
 		pc = (struct ide_atapi_pc *)rq->special;
 		break;
 	case REQ_TYPE_BLOCK_PC:
diff --git a/drivers/ide/ide-tape.c b/drivers/ide/ide-tape.c
index 2a5d543..f5d51d1 100644
--- a/drivers/ide/ide-tape.c
+++ b/drivers/ide/ide-tape.c
@@ -577,7 +577,7 @@ static ide_startstop_t idetape_do_request(ide_drive_t *drive,
 		      blk_rq_sectors(rq));
 
 	BUG_ON(!(rq->cmd_type == REQ_TYPE_DRV_PRIV ||
-		 rq->cmd_type == REQ_TYPE_SENSE));
+		 rq->cmd_type == REQ_TYPE_ATA_SENSE));
 
 	/* Retry a failed packet command */
 	if (drive->failed_pc && drive->pc->c[0] == REQUEST_SENSE) {
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 9cb4d80..6076b9e 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -75,7 +75,6 @@ struct request_list {
 enum rq_cmd_type_bits {
 	REQ_TYPE_FS		= 1,	/* fs request */
 	REQ_TYPE_BLOCK_PC,		/* scsi command */
-	REQ_TYPE_SENSE,			/* sense request */
 	REQ_TYPE_PM_SUSPEND,		/* suspend request */
 	REQ_TYPE_PM_RESUME,		/* resume request */
 	REQ_TYPE_PM_SHUTDOWN,		/* shutdown request */
diff --git a/include/linux/ide.h b/include/linux/ide.h
index 62ac399..9856b7d 100644
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -43,6 +43,7 @@ struct device;
 enum ata_cmd_type_bits {
 	REQ_TYPE_ATA_TASKFILE = REQ_TYPE_DRV_PRIV + 1,
 	REQ_TYPE_ATA_PC,
+	REQ_TYPE_ATA_SENSE,	/* sense request */
 };
 
 /* Error codes returned in rq->errors to the higher part of the driver. */
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/7] block: remove REQ_TYPE_PM_SHUTDOWN
  2015-04-17 20:37 RFC: struct request cleanups Christoph Hellwig
                   ` (2 preceding siblings ...)
  2015-04-17 20:37 ` [PATCH 3/7] block: move REQ_TYPE_SENSE to the ide driver Christoph Hellwig
@ 2015-04-17 20:37 ` Christoph Hellwig
  2015-04-17 20:37 ` [PATCH 5/7] block: move PM request support to IDE Christoph Hellwig
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2015-04-17 20:37 UTC (permalink / raw)
  To: axboe, linux-nvme; +Cc: linux-scsi

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 include/linux/blkdev.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 6076b9e..c2829ba 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -77,7 +77,6 @@ enum rq_cmd_type_bits {
 	REQ_TYPE_BLOCK_PC,		/* scsi command */
 	REQ_TYPE_PM_SUSPEND,		/* suspend request */
 	REQ_TYPE_PM_RESUME,		/* resume request */
-	REQ_TYPE_PM_SHUTDOWN,		/* shutdown request */
 	REQ_TYPE_DRV_PRIV,		/* driver defined types from here */
 };
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 5/7] block: move PM request support to IDE
  2015-04-17 20:37 RFC: struct request cleanups Christoph Hellwig
                   ` (3 preceding siblings ...)
  2015-04-17 20:37 ` [PATCH 4/7] block: remove REQ_TYPE_PM_SHUTDOWN Christoph Hellwig
@ 2015-04-17 20:37 ` Christoph Hellwig
  2015-04-17 20:37 ` [PATCH 6/7] nbd: stop using req->cmd Christoph Hellwig
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2015-04-17 20:37 UTC (permalink / raw)
  To: axboe, linux-nvme; +Cc: linux-scsi

This removes the request types and hacks from the block code and into the
old IDE driver.  There is a small amunt of code duplication due to this,
but it's not too bad.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-core.c           |  1 +
 block/blk-exec.c           | 10 ---------
 block/blk.h                |  2 --
 drivers/ide/ide-eh.c       |  2 +-
 drivers/ide/ide-io.c       |  8 +++----
 drivers/ide/ide-pm.c       | 56 +++++++++++++++++++++++++++++++++++-----------
 drivers/ide/ide-taskfile.c |  2 +-
 include/linux/blkdev.h     | 21 +----------------
 include/linux/ide.h        | 19 ++++++++++++++++
 9 files changed, 70 insertions(+), 51 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index fd154b9..2e5020f 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -285,6 +285,7 @@ inline void __blk_run_queue_uncond(struct request_queue *q)
 	q->request_fn(q);
 	q->request_fn_active--;
 }
+EXPORT_SYMBOL_GPL(__blk_run_queue_uncond);
 
 /**
  * __blk_run_queue - run a single device queue
diff --git a/block/blk-exec.c b/block/blk-exec.c
index 9924725..3fec8a2 100644
--- a/block/blk-exec.c
+++ b/block/blk-exec.c
@@ -53,7 +53,6 @@ void blk_execute_rq_nowait(struct request_queue *q, struct gendisk *bd_disk,
 			   rq_end_io_fn *done)
 {
 	int where = at_head ? ELEVATOR_INSERT_FRONT : ELEVATOR_INSERT_BACK;
-	bool is_pm_resume;
 
 	WARN_ON(irqs_disabled());
 	WARN_ON(rq->cmd_type == REQ_TYPE_FS);
@@ -70,12 +69,6 @@ void blk_execute_rq_nowait(struct request_queue *q, struct gendisk *bd_disk,
 		return;
 	}
 
-	/*
-	 * need to check this before __blk_run_queue(), because rq can
-	 * be freed before that returns.
-	 */
-	is_pm_resume = rq->cmd_type == REQ_TYPE_PM_RESUME;
-
 	spin_lock_irq(q->queue_lock);
 
 	if (unlikely(blk_queue_dying(q))) {
@@ -88,9 +81,6 @@ void blk_execute_rq_nowait(struct request_queue *q, struct gendisk *bd_disk,
 
 	__elv_add_request(q, rq, where);
 	__blk_run_queue(q);
-	/* the queue is stopped so it won't be run */
-	if (is_pm_resume)
-		__blk_run_queue_uncond(q);
 	spin_unlock_irq(q->queue_lock);
 }
 EXPORT_SYMBOL_GPL(blk_execute_rq_nowait);
diff --git a/block/blk.h b/block/blk.h
index 43b0361..4b48d55 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -193,8 +193,6 @@ int blk_try_merge(struct request *rq, struct bio *bio);
 
 void blk_queue_congestion_threshold(struct request_queue *q);
 
-void __blk_run_queue_uncond(struct request_queue *q);
-
 int blk_dev_init(void);
 
 
diff --git a/drivers/ide/ide-eh.c b/drivers/ide/ide-eh.c
index 19d809c..d6da011 100644
--- a/drivers/ide/ide-eh.c
+++ b/drivers/ide/ide-eh.c
@@ -129,7 +129,7 @@ ide_startstop_t ide_error(ide_drive_t *drive, const char *msg, u8 stat)
 
 			if (cmd)
 				ide_complete_cmd(drive, cmd, stat, err);
-		} else if (blk_pm_request(rq)) {
+		} else if (ata_pm_request(rq)) {
 			rq->errors = 1;
 			ide_complete_pm_rq(drive, rq);
 			return ide_stopped;
diff --git a/drivers/ide/ide-io.c b/drivers/ide/ide-io.c
index 8e55abd..669ea1e 100644
--- a/drivers/ide/ide-io.c
+++ b/drivers/ide/ide-io.c
@@ -320,7 +320,7 @@ static ide_startstop_t start_request (ide_drive_t *drive, struct request *rq)
 		goto kill_rq;
 	}
 
-	if (blk_pm_request(rq))
+	if (ata_pm_request(rq))
 		ide_check_pm_state(drive, rq);
 
 	drive->hwif->tp_ops->dev_select(drive);
@@ -342,8 +342,8 @@ static ide_startstop_t start_request (ide_drive_t *drive, struct request *rq)
 
 		if (rq->cmd_type == REQ_TYPE_ATA_TASKFILE)
 			return execute_drive_cmd(drive, rq);
-		else if (blk_pm_request(rq)) {
-			struct request_pm_state *pm = rq->special;
+		else if (ata_pm_request(rq)) {
+			struct ide_pm_state *pm = rq->special;
 #ifdef DEBUG_PM
 			printk("%s: start_power_step(step: %d)\n",
 				drive->name, pm->pm_step);
@@ -538,7 +538,7 @@ repeat:
 		 * state machine.
 		 */
 		if ((drive->dev_flags & IDE_DFLAG_BLOCKED) &&
-		    blk_pm_request(rq) == 0 &&
+		    ata_pm_request(rq) == 0 &&
 		    (rq->cmd_flags & REQ_PREEMPT) == 0) {
 			/* there should be no pending command at this point */
 			ide_unlock_port(hwif);
diff --git a/drivers/ide/ide-pm.c b/drivers/ide/ide-pm.c
index 8d1e32d..6a8bfed 100644
--- a/drivers/ide/ide-pm.c
+++ b/drivers/ide/ide-pm.c
@@ -8,7 +8,7 @@ int generic_ide_suspend(struct device *dev, pm_message_t mesg)
 	ide_drive_t *pair = ide_get_pair_dev(drive);
 	ide_hwif_t *hwif = drive->hwif;
 	struct request *rq;
-	struct request_pm_state rqpm;
+	struct ide_pm_state rqpm;
 	int ret;
 
 	if (ide_port_acpi(hwif)) {
@@ -19,7 +19,7 @@ int generic_ide_suspend(struct device *dev, pm_message_t mesg)
 
 	memset(&rqpm, 0, sizeof(rqpm));
 	rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
-	rq->cmd_type = REQ_TYPE_PM_SUSPEND;
+	rq->cmd_type = REQ_TYPE_ATA_PM_SUSPEND;
 	rq->special = &rqpm;
 	rqpm.pm_step = IDE_PM_START_SUSPEND;
 	if (mesg.event == PM_EVENT_PRETHAW)
@@ -38,13 +38,43 @@ int generic_ide_suspend(struct device *dev, pm_message_t mesg)
 	return ret;
 }
 
+static void ide_end_sync_rq(struct request *rq, int error)
+{
+	complete(rq->end_io_data);
+}
+
+static int ide_pm_execute_rq(struct request *rq)
+{
+	struct request_queue *q = rq->q;
+	DECLARE_COMPLETION_ONSTACK(wait);
+
+	rq->end_io_data = &wait;
+	rq->end_io = ide_end_sync_rq;
+
+	spin_lock_irq(q->queue_lock);
+	if (unlikely(blk_queue_dying(q))) {
+		rq->cmd_flags |= REQ_QUIET; 
+		rq->errors = -ENXIO;
+		__blk_end_request_all(rq, rq->errors);
+		spin_unlock_irq(q->queue_lock);
+		return -ENXIO;
+	}
+	__elv_add_request(q, rq, ELEVATOR_INSERT_FRONT);
+	__blk_run_queue_uncond(q);
+	spin_unlock_irq(q->queue_lock);
+
+	wait_for_completion_io(&wait);
+
+	return rq->errors ? -EIO : 0;
+}
+
 int generic_ide_resume(struct device *dev)
 {
 	ide_drive_t *drive = to_ide_device(dev);
 	ide_drive_t *pair = ide_get_pair_dev(drive);
 	ide_hwif_t *hwif = drive->hwif;
 	struct request *rq;
-	struct request_pm_state rqpm;
+	struct ide_pm_state rqpm;
 	int err;
 
 	if (ide_port_acpi(hwif)) {
@@ -59,13 +89,13 @@ int generic_ide_resume(struct device *dev)
 
 	memset(&rqpm, 0, sizeof(rqpm));
 	rq = blk_get_request(drive->queue, READ, __GFP_WAIT);
-	rq->cmd_type = REQ_TYPE_PM_RESUME;
+	rq->cmd_type = REQ_TYPE_ATA_PM_RESUME;
 	rq->cmd_flags |= REQ_PREEMPT;
 	rq->special = &rqpm;
 	rqpm.pm_step = IDE_PM_START_RESUME;
 	rqpm.pm_state = PM_EVENT_ON;
 
-	err = blk_execute_rq(drive->queue, NULL, rq, 1);
+	err = ide_pm_execute_rq(rq);
 	blk_put_request(rq);
 
 	if (err == 0 && dev->driver) {
@@ -80,7 +110,7 @@ int generic_ide_resume(struct device *dev)
 
 void ide_complete_power_step(ide_drive_t *drive, struct request *rq)
 {
-	struct request_pm_state *pm = rq->special;
+	struct ide_pm_state *pm = rq->special;
 
 #ifdef DEBUG_PM
 	printk(KERN_INFO "%s: complete_power_step(step: %d)\n",
@@ -110,7 +140,7 @@ void ide_complete_power_step(ide_drive_t *drive, struct request *rq)
 
 ide_startstop_t ide_start_power_step(ide_drive_t *drive, struct request *rq)
 {
-	struct request_pm_state *pm = rq->special;
+	struct ide_pm_state *pm = rq->special;
 	struct ide_cmd cmd = { };
 
 	switch (pm->pm_step) {
@@ -182,7 +212,7 @@ out_do_tf:
 void ide_complete_pm_rq(ide_drive_t *drive, struct request *rq)
 {
 	struct request_queue *q = drive->queue;
-	struct request_pm_state *pm = rq->special;
+	struct ide_pm_state *pm = rq->special;
 	unsigned long flags;
 
 	ide_complete_power_step(drive, rq);
@@ -191,10 +221,10 @@ void ide_complete_pm_rq(ide_drive_t *drive, struct request *rq)
 
 #ifdef DEBUG_PM
 	printk("%s: completing PM request, %s\n", drive->name,
-	       (rq->cmd_type == REQ_TYPE_PM_SUSPEND) ? "suspend" : "resume");
+	       (rq->cmd_type == REQ_TYPE_ATA_PM_SUSPEND) ? "suspend" : "resume");
 #endif
 	spin_lock_irqsave(q->queue_lock, flags);
-	if (rq->cmd_type == REQ_TYPE_PM_SUSPEND)
+	if (rq->cmd_type == REQ_TYPE_ATA_PM_SUSPEND)
 		blk_stop_queue(q);
 	else
 		drive->dev_flags &= ~IDE_DFLAG_BLOCKED;
@@ -208,13 +238,13 @@ void ide_complete_pm_rq(ide_drive_t *drive, struct request *rq)
 
 void ide_check_pm_state(ide_drive_t *drive, struct request *rq)
 {
-	struct request_pm_state *pm = rq->special;
+	struct ide_pm_state *pm = rq->special;
 
-	if (rq->cmd_type == REQ_TYPE_PM_SUSPEND &&
+	if (rq->cmd_type == REQ_TYPE_ATA_PM_SUSPEND &&
 	    pm->pm_step == IDE_PM_START_SUSPEND)
 		/* Mark drive blocked when starting the suspend sequence. */
 		drive->dev_flags |= IDE_DFLAG_BLOCKED;
-	else if (rq->cmd_type == REQ_TYPE_PM_RESUME &&
+	else if (rq->cmd_type == REQ_TYPE_ATA_PM_RESUME &&
 		 pm->pm_step == IDE_PM_START_RESUME) {
 		/*
 		 * The first thing we do on wakeup is to wait for BSY bit to
diff --git a/drivers/ide/ide-taskfile.c b/drivers/ide/ide-taskfile.c
index dabb88b..0979e12 100644
--- a/drivers/ide/ide-taskfile.c
+++ b/drivers/ide/ide-taskfile.c
@@ -186,7 +186,7 @@ static ide_startstop_t task_no_data_intr(ide_drive_t *drive)
 	    tf->command == ATA_CMD_CHK_POWER) {
 		struct request *rq = hwif->rq;
 
-		if (blk_pm_request(rq))
+		if (ata_pm_request(rq))
 			ide_complete_pm_rq(drive, rq);
 		else
 			ide_finish_cmd(drive, cmd, stat);
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index c2829ba..2da818a 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -30,7 +30,6 @@ struct scsi_ioctl_command;
 
 struct request_queue;
 struct elevator_queue;
-struct request_pm_state;
 struct blk_trace;
 struct request;
 struct sg_io_hdr;
@@ -75,8 +74,6 @@ struct request_list {
 enum rq_cmd_type_bits {
 	REQ_TYPE_FS		= 1,	/* fs request */
 	REQ_TYPE_BLOCK_PC,		/* scsi command */
-	REQ_TYPE_PM_SUSPEND,		/* suspend request */
-	REQ_TYPE_PM_RESUME,		/* resume request */
 	REQ_TYPE_DRV_PRIV,		/* driver defined types from here */
 };
 
@@ -207,19 +204,6 @@ static inline unsigned short req_get_ioprio(struct request *req)
 	return req->ioprio;
 }
 
-/*
- * State information carried for REQ_TYPE_PM_SUSPEND and REQ_TYPE_PM_RESUME
- * requests. Some step values could eventually be made generic.
- */
-struct request_pm_state
-{
-	/* PM state machine step value, currently driver specific */
-	int	pm_step;
-	/* requested PM state value (S1, S2, S3, S4, ...) */
-	u32	pm_state;
-	void*	data;		/* for driver use */
-};
-
 #include <linux/elevator.h>
 
 struct blk_queue_ctx;
@@ -601,10 +585,6 @@ static inline void queue_flag_clear(unsigned int flag, struct request_queue *q)
 	(((rq)->cmd_flags & REQ_STARTED) && \
 	 ((rq)->cmd_type == REQ_TYPE_FS))
 
-#define blk_pm_request(rq)	\
-	((rq)->cmd_type == REQ_TYPE_PM_SUSPEND || \
-	 (rq)->cmd_type == REQ_TYPE_PM_RESUME)
-
 #define blk_rq_cpu_valid(rq)	((rq)->cpu != -1)
 #define blk_bidi_rq(rq)		((rq)->next_rq != NULL)
 /* rq->queuelist of dequeued request must be list_empty() */
@@ -838,6 +818,7 @@ extern void blk_stop_queue(struct request_queue *q);
 extern void blk_sync_queue(struct request_queue *q);
 extern void __blk_stop_queue(struct request_queue *q);
 extern void __blk_run_queue(struct request_queue *q);
+extern void __blk_run_queue_uncond(struct request_queue *q);
 extern void blk_run_queue(struct request_queue *);
 extern void blk_run_queue_async(struct request_queue *q);
 extern int blk_rq_map_user(struct request_queue *, struct request *,
diff --git a/include/linux/ide.h b/include/linux/ide.h
index 9856b7d..a633898 100644
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -44,8 +44,14 @@ enum ata_cmd_type_bits {
 	REQ_TYPE_ATA_TASKFILE = REQ_TYPE_DRV_PRIV + 1,
 	REQ_TYPE_ATA_PC,
 	REQ_TYPE_ATA_SENSE,	/* sense request */
+	REQ_TYPE_ATA_PM_SUSPEND,/* suspend request */
+	REQ_TYPE_ATA_PM_RESUME,	/* resume request */
 };
 
+#define ata_pm_request(rq)	\
+	((rq)->cmd_type == REQ_TYPE_ATA_PM_SUSPEND || \
+	 (rq)->cmd_type == REQ_TYPE_ATA_PM_RESUME)
+
 /* Error codes returned in rq->errors to the higher part of the driver. */
 enum {
 	IDE_DRV_ERROR_GENERAL	= 101,
@@ -1321,6 +1327,19 @@ struct ide_port_info {
 	u8			udma_mask;
 };
 
+/*
+ * State information carried for REQ_TYPE_ATA_PM_SUSPEND and REQ_TYPE_ATA_PM_RESUME
+ * requests.
+ */
+struct ide_pm_state {
+	/* PM state machine step value, currently driver specific */
+	int	pm_step;
+	/* requested PM state value (S1, S2, S3, S4, ...) */
+	u32	pm_state;
+	void*	data;		/* for driver use */
+};
+
+
 int ide_pci_init_one(struct pci_dev *, const struct ide_port_info *, void *);
 int ide_pci_init_two(struct pci_dev *, struct pci_dev *,
 		     const struct ide_port_info *, void *);
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 6/7] nbd: stop using req->cmd
  2015-04-17 20:37 RFC: struct request cleanups Christoph Hellwig
                   ` (4 preceding siblings ...)
  2015-04-17 20:37 ` [PATCH 5/7] block: move PM request support to IDE Christoph Hellwig
@ 2015-04-17 20:37 ` Christoph Hellwig
  2015-04-17 20:37 ` [PATCH 7/7] block: allocate block_pc data separate from struct request Christoph Hellwig
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2015-04-17 20:37 UTC (permalink / raw)
  To: axboe, linux-nvme; +Cc: linux-scsi

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/block/nbd.c      | 48 +++++++++++++++++++++++-------------------------
 include/uapi/linux/nbd.h |  2 --
 2 files changed, 23 insertions(+), 27 deletions(-)

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 9cf52ac..83a7ba4 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -230,29 +230,40 @@ static int nbd_send_req(struct nbd_device *nbd, struct request *req)
 	int result, flags;
 	struct nbd_request request;
 	unsigned long size = blk_rq_bytes(req);
+	u32 type;
+
+	if (req->cmd_type == REQ_TYPE_DRV_PRIV)
+		type = NBD_CMD_DISC;
+	else if (req->cmd_flags & REQ_DISCARD)
+		type = NBD_CMD_TRIM;
+	else if (req->cmd_flags & REQ_FLUSH)
+		type = NBD_CMD_FLUSH;
+	else if (rq_data_dir(req) == WRITE)
+		type = NBD_CMD_WRITE;
+	else
+		type = NBD_CMD_READ;
 
 	memset(&request, 0, sizeof(request));
 	request.magic = htonl(NBD_REQUEST_MAGIC);
-	request.type = htonl(nbd_cmd(req));
-
-	if (nbd_cmd(req) != NBD_CMD_FLUSH && nbd_cmd(req) != NBD_CMD_DISC) {
+	request.type = htonl(type);
+	if (type != NBD_CMD_FLUSH && type != NBD_CMD_DISC) {
 		request.from = cpu_to_be64((u64)blk_rq_pos(req) << 9);
 		request.len = htonl(size);
 	}
 	memcpy(request.handle, &req, sizeof(req));
 
 	dev_dbg(nbd_to_dev(nbd), "request %p: sending control (%s@%llu,%uB)\n",
-		req, nbdcmd_to_ascii(nbd_cmd(req)),
+		req, nbdcmd_to_ascii(type),
 		(unsigned long long)blk_rq_pos(req) << 9, blk_rq_bytes(req));
 	result = sock_xmit(nbd, 1, &request, sizeof(request),
-			(nbd_cmd(req) == NBD_CMD_WRITE) ? MSG_MORE : 0);
+			(type == NBD_CMD_WRITE) ? MSG_MORE : 0);
 	if (result <= 0) {
 		dev_err(disk_to_dev(nbd->disk),
 			"Send control failed (result %d)\n", result);
 		return -EIO;
 	}
 
-	if (nbd_cmd(req) == NBD_CMD_WRITE) {
+	if (type == NBD_CMD_WRITE) {
 		struct req_iterator iter;
 		struct bio_vec bvec;
 		/*
@@ -352,7 +363,7 @@ static struct request *nbd_read_stat(struct nbd_device *nbd)
 	}
 
 	dev_dbg(nbd_to_dev(nbd), "request %p: got reply\n", req);
-	if (nbd_cmd(req) == NBD_CMD_READ) {
+	if (rq_data_dir(req) != WRITE) {
 		struct req_iterator iter;
 		struct bio_vec bvec;
 
@@ -452,23 +463,11 @@ static void nbd_handle_req(struct nbd_device *nbd, struct request *req)
 	if (req->cmd_type != REQ_TYPE_FS)
 		goto error_out;
 
-	nbd_cmd(req) = NBD_CMD_READ;
-	if (rq_data_dir(req) == WRITE) {
-		if ((req->cmd_flags & REQ_DISCARD)) {
-			WARN_ON(!(nbd->flags & NBD_FLAG_SEND_TRIM));
-			nbd_cmd(req) = NBD_CMD_TRIM;
-		} else
-			nbd_cmd(req) = NBD_CMD_WRITE;
-		if (nbd->flags & NBD_FLAG_READ_ONLY) {
-			dev_err(disk_to_dev(nbd->disk),
-				"Write on read-only\n");
-			goto error_out;
-		}
-	}
-
-	if (req->cmd_flags & REQ_FLUSH) {
-		BUG_ON(unlikely(blk_rq_sectors(req)));
-		nbd_cmd(req) = NBD_CMD_FLUSH;
+	if (rq_data_dir(req) == WRITE &&
+	    (nbd->flags & NBD_FLAG_READ_ONLY)) {
+		dev_err(disk_to_dev(nbd->disk),
+			"Write on read-only\n");
+		goto error_out;
 	}
 
 	req->errors = 0;
@@ -593,7 +592,6 @@ static int __nbd_ioctl(struct block_device *bdev, struct nbd_device *nbd,
 		mutex_lock(&nbd->tx_lock);
 		blk_rq_init(NULL, &sreq);
 		sreq.cmd_type = REQ_TYPE_DRV_PRIV;
-		nbd_cmd(&sreq) = NBD_CMD_DISC;
 
 		/* Check again after getting mutex back.  */
 		if (!nbd->sock)
diff --git a/include/uapi/linux/nbd.h b/include/uapi/linux/nbd.h
index 4f52549..e08e413 100644
--- a/include/uapi/linux/nbd.h
+++ b/include/uapi/linux/nbd.h
@@ -44,8 +44,6 @@ enum {
 /* there is a gap here to match userspace */
 #define NBD_FLAG_SEND_TRIM    (1 << 5) /* send trim/discard */
 
-#define nbd_cmd(req) ((req)->cmd[0])
-
 /* userspace doesn't need the nbd_device structure */
 
 /* These are sent over the network in the request/reply magic fields */
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 7/7] block: allocate block_pc data separate from struct request
  2015-04-17 20:37 RFC: struct request cleanups Christoph Hellwig
                   ` (5 preceding siblings ...)
  2015-04-17 20:37 ` [PATCH 6/7] nbd: stop using req->cmd Christoph Hellwig
@ 2015-04-17 20:37 ` Christoph Hellwig
  2015-05-06 11:46   ` Boaz Harrosh
  2015-04-21 19:51 ` RFC: struct request cleanups Matthew Wilcox
  2015-05-05 19:41 ` Jens Axboe
  8 siblings, 1 reply; 12+ messages in thread
From: Christoph Hellwig @ 2015-04-17 20:37 UTC (permalink / raw)
  To: axboe, linux-nvme; +Cc: linux-scsi

Don't bloat struct request with BLOCK_PC specific fields.

WIP, breaks dm BLOCK_PC passthrough and the old IDE driver for now.
---
 block/blk-core.c                            | 23 ++++++---
 block/blk-exec.c                            | 17 -------
 block/blk-mq.c                              |  4 --
 block/bsg-lib.c                             | 10 ++--
 block/bsg.c                                 | 31 ++++--------
 block/scsi_ioctl.c                          | 68 +++++++++++--------------
 drivers/ata/libata-scsi.c                   |  2 +-
 drivers/block/pktcdvd.c                     |  9 ++--
 drivers/block/virtio_blk.c                  |  9 ++--
 drivers/cdrom/cdrom.c                       | 36 +++++++------
 drivers/md/dm.c                             |  4 ++
 drivers/message/fusion/mptsas.c             |  4 +-
 drivers/scsi/device_handler/scsi_dh_alua.c  | 78 ++++++++++++++---------------
 drivers/scsi/device_handler/scsi_dh_emc.c   | 52 +++++++++----------
 drivers/scsi/device_handler/scsi_dh_hp_sw.c | 38 ++++++++------
 drivers/scsi/device_handler/scsi_dh_rdac.c  | 43 ++++++++--------
 drivers/scsi/mpt2sas/mpt2sas_transport.c    |  4 +-
 drivers/scsi/mpt3sas/mpt3sas_transport.c    |  4 +-
 drivers/scsi/osd/osd_initiator.c            | 32 ++++++------
 drivers/scsi/osst.c                         | 10 ++--
 drivers/scsi/qla2xxx/qla_bsg.c              |  2 +-
 drivers/scsi/qla2xxx/qla_isr.c              |  6 ++-
 drivers/scsi/qla2xxx/qla_mr.c               |  2 +-
 drivers/scsi/scsi_error.c                   | 22 ++++----
 drivers/scsi/scsi_lib.c                     | 22 ++++----
 drivers/scsi/scsi_transport_fc.c            | 10 ++--
 drivers/scsi/sd.c                           | 47 ++---------------
 drivers/scsi/sg.c                           | 21 +++-----
 drivers/scsi/st.c                           | 21 ++++----
 drivers/target/target_core_pscsi.c          | 14 +++---
 include/linux/blkdev.h                      | 27 ++++++----
 include/linux/blktrace_api.h                |  4 +-
 include/scsi/scsi_cmnd.h                    |  2 +-
 kernel/trace/blktrace.c                     | 11 ++--
 34 files changed, 319 insertions(+), 370 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 2e5020f..5d78a85 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -105,8 +105,6 @@ void blk_rq_init(struct request_queue *q, struct request *rq)
 	rq->__sector = (sector_t) -1;
 	INIT_HLIST_NODE(&rq->hash);
 	RB_CLEAR_NODE(&rq->rb_node);
-	rq->cmd = rq->__cmd;
-	rq->cmd_len = BLK_MAX_CDB;
 	rq->tag = -1;
 	rq->start_time = jiffies;
 	set_start_time_ns(rq);
@@ -149,7 +147,7 @@ void blk_dump_rq_flags(struct request *rq, char *msg)
 	if (rq->cmd_type == REQ_TYPE_BLOCK_PC) {
 		printk(KERN_INFO "  cdb: ");
 		for (bit = 0; bit < BLK_MAX_CDB; bit++)
-			printk("%02x ", rq->cmd[bit]);
+			printk("%02x ", rq->block_pc->cmd[bit]);
 		printk("\n");
 	}
 }
@@ -1253,8 +1251,6 @@ struct request *blk_make_request(struct request_queue *q, struct bio *bio,
 	if (IS_ERR(rq))
 		return rq;
 
-	blk_rq_set_block_pc(rq);
-
 	for_each_bio(bio) {
 		struct bio *bounce_bio = bio;
 		int ret;
@@ -1274,15 +1270,24 @@ EXPORT_SYMBOL(blk_make_request);
 /**
  * blk_rq_set_block_pc - initialize a request to type BLOCK_PC
  * @rq:		request to be initialized
+ * @cmd_len:	length of the CDB
+ * @gfp:	kmalloc flags
  *
  */
-void blk_rq_set_block_pc(struct request *rq)
+int blk_rq_set_block_pc(struct request *rq, unsigned short cmd_len,
+		u8 *sense, gfp_t gfp)
 {
 	rq->cmd_type = REQ_TYPE_BLOCK_PC;
 	rq->__data_len = 0;
 	rq->__sector = (sector_t) -1;
 	rq->bio = rq->biotail = NULL;
-	memset(rq->__cmd, 0, sizeof(rq->__cmd));
+
+	rq->block_pc = kzalloc(sizeof(*rq->block_pc) + cmd_len, gfp);
+	if (!rq->block_pc)
+		return -ENOMEM;
+	rq->block_pc->cmd_len = cmd_len;
+	rq->block_pc->sense = sense;
+	return 0;
 }
 EXPORT_SYMBOL(blk_rq_set_block_pc);
 
@@ -1379,6 +1384,10 @@ void __blk_put_request(struct request_queue *q, struct request *req)
 	if (unlikely(!q))
 		return;
 
+	/* could also be other type-specific data */
+	if (req->block_pc)
+		kfree(req->block_pc);
+
 	if (q->mq_ops) {
 		blk_mq_free_request(req);
 		return;
diff --git a/block/blk-exec.c b/block/blk-exec.c
index 3fec8a2..94e909e 100644
--- a/block/blk-exec.c
+++ b/block/blk-exec.c
@@ -10,11 +10,6 @@
 
 #include "blk.h"
 
-/*
- * for max sense size
- */
-#include <scsi/scsi_cmnd.h>
-
 /**
  * blk_end_sync_rq - executes a completion event on a request
  * @rq: request to complete
@@ -100,16 +95,9 @@ int blk_execute_rq(struct request_queue *q, struct gendisk *bd_disk,
 		   struct request *rq, int at_head)
 {
 	DECLARE_COMPLETION_ONSTACK(wait);
-	char sense[SCSI_SENSE_BUFFERSIZE];
 	int err = 0;
 	unsigned long hang_check;
 
-	if (!rq->sense) {
-		memset(sense, 0, sizeof(sense));
-		rq->sense = sense;
-		rq->sense_len = 0;
-	}
-
 	rq->end_io_data = &wait;
 	blk_execute_rq_nowait(q, bd_disk, rq, at_head, blk_end_sync_rq);
 
@@ -123,11 +111,6 @@ int blk_execute_rq(struct request_queue *q, struct gendisk *bd_disk,
 	if (rq->errors)
 		err = -EIO;
 
-	if (rq->sense == sense)	{
-		rq->sense = NULL;
-		rq->sense_len = 0;
-	}
-
 	return err;
 }
 EXPORT_SYMBOL(blk_execute_rq);
diff --git a/block/blk-mq.c b/block/blk-mq.c
index ade8a2d..715e3c0 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -210,12 +210,8 @@ static void blk_mq_rq_ctx_init(struct request_queue *q, struct blk_mq_ctx *ctx,
 	/* tag was already set */
 	rq->errors = 0;
 
-	rq->cmd = rq->__cmd;
-
 	rq->extra_len = 0;
-	rq->sense_len = 0;
 	rq->resid_len = 0;
-	rq->sense = NULL;
 
 	INIT_LIST_HEAD(&rq->timeout_list);
 	rq->timeout = 0;
diff --git a/block/bsg-lib.c b/block/bsg-lib.c
index 650f427..6b73dca 100644
--- a/block/bsg-lib.c
+++ b/block/bsg-lib.c
@@ -59,9 +59,9 @@ void bsg_job_done(struct bsg_job *job, int result,
 	err = job->req->errors = result;
 	if (err < 0)
 		/* we're only returning the result field in the reply */
-		job->req->sense_len = sizeof(u32);
+		job->req->block_pc->sense_len = sizeof(u32);
 	else
-		job->req->sense_len = job->reply_len;
+		job->req->block_pc->sense_len = job->reply_len;
 	/* we assume all request payload was transferred, residual == 0 */
 	req->resid_len = 0;
 
@@ -124,9 +124,9 @@ static int bsg_create_job(struct device *dev, struct request *req)
 	job->req = req;
 	if (q->bsg_job_size)
 		job->dd_data = (void *)&job[1];
-	job->request = req->cmd;
-	job->request_len = req->cmd_len;
-	job->reply = req->sense;
+	job->request = req->block_pc->cmd;
+	job->request_len = req->block_pc->cmd_len;
+	job->reply = req->block_pc->sense;
 	job->reply_len = SCSI_SENSE_BUFFERSIZE;	/* Size of sense buffer
 						 * allocated */
 	if (req->bio) {
diff --git a/block/bsg.c b/block/bsg.c
index d214e92..ebf0dc1 100644
--- a/block/bsg.c
+++ b/block/bsg.c
@@ -140,18 +140,13 @@ static int blk_fill_sgv4_hdr_rq(struct request_queue *q, struct request *rq,
 				struct sg_io_v4 *hdr, struct bsg_device *bd,
 				fmode_t has_write_perm)
 {
-	if (hdr->request_len > BLK_MAX_CDB) {
-		rq->cmd = kzalloc(hdr->request_len, GFP_KERNEL);
-		if (!rq->cmd)
-			return -ENOMEM;
-	}
-
-	if (copy_from_user(rq->cmd, (void __user *)(unsigned long)hdr->request,
+	if (copy_from_user(rq->block_pc->cmd,
+			   (void __user *)(unsigned long)hdr->request,
 			   hdr->request_len))
 		return -EFAULT;
 
 	if (hdr->subprotocol == BSG_SUB_PROTOCOL_SCSI_CMD) {
-		if (blk_verify_command(rq->cmd, has_write_perm))
+		if (blk_verify_command(rq->block_pc->cmd, has_write_perm))
 			return -EPERM;
 	} else if (!capable(CAP_SYS_RAWIO))
 		return -EPERM;
@@ -159,8 +154,6 @@ static int blk_fill_sgv4_hdr_rq(struct request_queue *q, struct request *rq,
 	/*
 	 * fill in request structure
 	 */
-	rq->cmd_len = hdr->request_len;
-
 	rq->timeout = msecs_to_jiffies(hdr->timeout);
 	if (!rq->timeout)
 		rq->timeout = q->sg_timeout;
@@ -236,7 +229,10 @@ bsg_map_hdr(struct bsg_device *bd, struct sg_io_v4 *hdr, fmode_t has_write_perm,
 	rq = blk_get_request(q, rw, GFP_KERNEL);
 	if (IS_ERR(rq))
 		return rq;
-	blk_rq_set_block_pc(rq);
+
+	ret = blk_rq_set_block_pc(rq, hdr->request_len, sense, GFP_KERNEL);
+	if (ret)
+		goto out;
 
 	ret = blk_fill_sgv4_hdr_rq(q, rq, hdr, bd, has_write_perm);
 	if (ret)
@@ -280,13 +276,8 @@ bsg_map_hdr(struct bsg_device *bd, struct sg_io_v4 *hdr, fmode_t has_write_perm,
 			goto out;
 	}
 
-	rq->sense = sense;
-	rq->sense_len = 0;
-
 	return rq;
 out:
-	if (rq->cmd != rq->__cmd)
-		kfree(rq->cmd);
 	blk_put_request(rq);
 	if (next_rq) {
 		blk_rq_unmap_user(next_rq->bio);
@@ -407,12 +398,12 @@ static int blk_complete_sgv4_hdr_rq(struct request *rq, struct sg_io_v4 *hdr,
 		hdr->info |= SG_INFO_CHECK;
 	hdr->response_len = 0;
 
-	if (rq->sense_len && hdr->response) {
+	if (rq->block_pc->sense_len && hdr->response) {
 		int len = min_t(unsigned int, hdr->max_response_len,
-					rq->sense_len);
+					rq->block_pc->sense_len);
 
 		ret = copy_to_user((void __user *)(unsigned long)hdr->response,
-				   rq->sense, len);
+				   rq->block_pc->sense, len);
 		if (!ret)
 			hdr->response_len = len;
 		else
@@ -439,8 +430,6 @@ static int blk_complete_sgv4_hdr_rq(struct request *rq, struct sg_io_v4 *hdr,
 		ret = rq->errors;
 
 	blk_rq_unmap_user(bio);
-	if (rq->cmd != rq->__cmd)
-		kfree(rq->cmd);
 	blk_put_request(rq);
 
 	return ret;
diff --git a/block/scsi_ioctl.c b/block/scsi_ioctl.c
index 55b6f15..62f4e16 100644
--- a/block/scsi_ioctl.c
+++ b/block/scsi_ioctl.c
@@ -227,16 +227,14 @@ EXPORT_SYMBOL(blk_verify_command);
 static int blk_fill_sghdr_rq(struct request_queue *q, struct request *rq,
 			     struct sg_io_hdr *hdr, fmode_t mode)
 {
-	if (copy_from_user(rq->cmd, hdr->cmdp, hdr->cmd_len))
+	if (copy_from_user(rq->block_pc->cmd, hdr->cmdp, hdr->cmd_len))
 		return -EFAULT;
-	if (blk_verify_command(rq->cmd, mode & FMODE_WRITE))
+	if (blk_verify_command(rq->block_pc->cmd, mode & FMODE_WRITE))
 		return -EPERM;
 
 	/*
 	 * fill in request structure
 	 */
-	rq->cmd_len = hdr->cmd_len;
-
 	rq->timeout = msecs_to_jiffies(hdr->timeout);
 	if (!rq->timeout)
 		rq->timeout = q->sg_timeout;
@@ -267,10 +265,11 @@ static int blk_complete_sghdr_rq(struct request *rq, struct sg_io_hdr *hdr,
 	hdr->resid = rq->resid_len;
 	hdr->sb_len_wr = 0;
 
-	if (rq->sense_len && hdr->sbp) {
-		int len = min((unsigned int) hdr->mx_sb_len, rq->sense_len);
+	if (rq->block_pc->sense_len && hdr->sbp) {
+		int len = min((unsigned int) hdr->mx_sb_len,
+				rq->block_pc->sense_len);
 
-		if (!copy_to_user(hdr->sbp, rq->sense, len))
+		if (!copy_to_user(hdr->sbp, rq->block_pc->sense, len))
 			hdr->sb_len_wr = len;
 		else
 			ret = -EFAULT;
@@ -291,7 +290,6 @@ static int sg_io(struct request_queue *q, struct gendisk *bd_disk,
 	int writing = 0;
 	int at_head = 0;
 	struct request *rq;
-	char sense[SCSI_SENSE_BUFFERSIZE];
 	struct bio *bio;
 
 	if (hdr->interface_id != 'S')
@@ -318,17 +316,14 @@ static int sg_io(struct request_queue *q, struct gendisk *bd_disk,
 	rq = blk_get_request(q, writing ? WRITE : READ, GFP_KERNEL);
 	if (IS_ERR(rq))
 		return PTR_ERR(rq);
-	blk_rq_set_block_pc(rq);
 
-	if (hdr->cmd_len > BLK_MAX_CDB) {
-		rq->cmd = kzalloc(hdr->cmd_len, GFP_KERNEL);
-		if (!rq->cmd)
-			goto out_put_request;
-	}
+	ret = blk_rq_set_block_pc(rq, hdr->cmd_len, NULL, GFP_KERNEL);
+	if (ret)
+		goto out_put_request;
 
 	ret = -EFAULT;
 	if (blk_fill_sghdr_rq(q, rq, hdr, mode))
-		goto out_free_cdb;
+		goto out_put_request;
 
 	ret = 0;
 	if (hdr->iovec_count) {
@@ -339,7 +334,7 @@ static int sg_io(struct request_queue *q, struct gendisk *bd_disk,
 				   hdr->dxferp, hdr->iovec_count,
 				   0, &iov, &i);
 		if (ret < 0)
-			goto out_free_cdb;
+			goto out_put_request;
 
 		/* SG_IO howto says that the shorter of the two wins */
 		iov_iter_truncate(&i, hdr->dxfer_len);
@@ -351,12 +346,9 @@ static int sg_io(struct request_queue *q, struct gendisk *bd_disk,
 				      GFP_KERNEL);
 
 	if (ret)
-		goto out_free_cdb;
+		goto out_put_request;
 
 	bio = rq->bio;
-	memset(sense, 0, sizeof(sense));
-	rq->sense = sense;
-	rq->sense_len = 0;
 	rq->retries = 0;
 
 	start_time = jiffies;
@@ -371,9 +363,6 @@ static int sg_io(struct request_queue *q, struct gendisk *bd_disk,
 
 	ret = blk_complete_sghdr_rq(rq, hdr, bio);
 
-out_free_cdb:
-	if (rq->cmd != rq->__cmd)
-		kfree(rq->cmd);
 out_put_request:
 	blk_put_request(rq);
 	return ret;
@@ -449,22 +438,23 @@ int sg_scsi_ioctl(struct request_queue *q, struct gendisk *disk, fmode_t mode,
 		err = PTR_ERR(rq);
 		goto error_free_buffer;
 	}
-	blk_rq_set_block_pc(rq);
 
 	cmdlen = COMMAND_SIZE(opcode);
+	err = blk_rq_set_block_pc(rq, cmdlen, sense, GFP_KERNEL);
+	if (err)
+		goto error;
 
 	/*
 	 * get command and data to send to device, if any
 	 */
 	err = -EFAULT;
-	rq->cmd_len = cmdlen;
-	if (copy_from_user(rq->cmd, sic->data, cmdlen))
+	if (copy_from_user(rq->block_pc->cmd, sic->data, cmdlen))
 		goto error;
 
 	if (in_len && copy_from_user(buffer, sic->data + cmdlen, in_len))
 		goto error;
 
-	err = blk_verify_command(rq->cmd, mode & FMODE_WRITE);
+	err = blk_verify_command(rq->block_pc->cmd, mode & FMODE_WRITE);
 	if (err)
 		goto error;
 
@@ -500,18 +490,14 @@ int sg_scsi_ioctl(struct request_queue *q, struct gendisk *disk, fmode_t mode,
 		goto error;
 	}
 
-	memset(sense, 0, sizeof(sense));
-	rq->sense = sense;
-	rq->sense_len = 0;
-
 	blk_execute_rq(q, disk, rq, 0);
 
 	err = rq->errors & 0xff;	/* only 8 bit SCSI status */
 	if (err) {
-		if (rq->sense_len && rq->sense) {
-			bytes = (OMAX_SB_LEN > rq->sense_len) ?
-				rq->sense_len : OMAX_SB_LEN;
-			if (copy_to_user(sic->data, rq->sense, bytes))
+		if (rq->block_pc->sense_len) {
+			bytes = (OMAX_SB_LEN > rq->block_pc->sense_len) ?
+				rq->block_pc->sense_len : OMAX_SB_LEN;
+			if (copy_to_user(sic->data, rq->block_pc->sense, bytes))
 				err = -EFAULT;
 		}
 	} else {
@@ -539,14 +525,16 @@ static int __blk_send_generic(struct request_queue *q, struct gendisk *bd_disk,
 	rq = blk_get_request(q, WRITE, __GFP_WAIT);
 	if (IS_ERR(rq))
 		return PTR_ERR(rq);
-	blk_rq_set_block_pc(rq);
+	err = blk_rq_set_block_pc(rq, 6, NULL, GFP_KERNEL);
+	if (err)
+		goto out_put_request;
+
 	rq->timeout = BLK_DEFAULT_SG_TIMEOUT;
-	rq->cmd[0] = cmd;
-	rq->cmd[4] = data;
-	rq->cmd_len = 6;
+	rq->block_pc->cmd[0] = cmd;
+	rq->block_pc->cmd[4] = data;
 	err = blk_execute_rq(q, bd_disk, rq, 0);
+out_put_request:
 	blk_put_request(rq);
-
 	return err;
 }
 
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 3131adc..d8cb4ec 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1143,7 +1143,7 @@ static int atapi_drain_needed(struct request *rq)
 	if (!blk_rq_bytes(rq) || (rq->cmd_flags & REQ_WRITE))
 		return 0;
 
-	return atapi_cmd_type(rq->cmd[0]) == ATAPI_MISC;
+	return atapi_cmd_type(rq->block_pc->cmd[0]) == ATAPI_MISC;
 }
 
 static int ata_scsi_dev_config(struct scsi_device *sdev,
diff --git a/drivers/block/pktcdvd.c b/drivers/block/pktcdvd.c
index 09e628da..51f919d 100644
--- a/drivers/block/pktcdvd.c
+++ b/drivers/block/pktcdvd.c
@@ -706,7 +706,11 @@ static int pkt_generic_packet(struct pktcdvd_device *pd, struct packet_command *
 			     WRITE : READ, __GFP_WAIT);
 	if (IS_ERR(rq))
 		return PTR_ERR(rq);
-	blk_rq_set_block_pc(rq);
+
+	ret = blk_rq_set_block_pc(rq, COMMAND_SIZE(cgc->cmd[0]), NULL,
+			GFP_KERNEL);
+	if (ret)
+		goto out;
 
 	if (cgc->buflen) {
 		ret = blk_rq_map_kern(q, rq, cgc->buffer, cgc->buflen,
@@ -715,8 +719,7 @@ static int pkt_generic_packet(struct pktcdvd_device *pd, struct packet_command *
 			goto out;
 	}
 
-	rq->cmd_len = COMMAND_SIZE(cgc->cmd[0]);
-	memcpy(rq->cmd, cgc->cmd, CDROM_PACKET_SIZE);
+	memcpy(rq->block_pc->cmd, cgc->cmd, CDROM_PACKET_SIZE);
 
 	rq->timeout = 60*HZ;
 	if (cgc->quiet)
diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index d4d05f0..cd3bcc0 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -90,7 +90,8 @@ static int __virtblk_add_req(struct virtqueue *vq,
 	 * inhdr with additional status information.
 	 */
 	if (type == cpu_to_virtio32(vq->vdev, VIRTIO_BLK_T_SCSI_CMD)) {
-		sg_init_one(&cmd, vbr->req->cmd, vbr->req->cmd_len);
+		sg_init_one(&cmd, vbr->req->block_pc->cmd,
+				vbr->req->block_pc->cmd_len);
 		sgs[num_out++] = &cmd;
 	}
 
@@ -102,7 +103,8 @@ static int __virtblk_add_req(struct virtqueue *vq,
 	}
 
 	if (type == cpu_to_virtio32(vq->vdev, VIRTIO_BLK_T_SCSI_CMD)) {
-		sg_init_one(&sense, vbr->req->sense, SCSI_SENSE_BUFFERSIZE);
+		sg_init_one(&sense, vbr->req->block_pc->sense,
+				SCSI_SENSE_BUFFERSIZE);
 		sgs[num_out + num_in++] = &sense;
 		sg_init_one(&inhdr, &vbr->in_hdr, sizeof(vbr->in_hdr));
 		sgs[num_out + num_in++] = &inhdr;
@@ -122,7 +124,8 @@ static inline void virtblk_request_done(struct request *req)
 
 	if (req->cmd_type == REQ_TYPE_BLOCK_PC) {
 		req->resid_len = virtio32_to_cpu(vblk->vdev, vbr->in_hdr.residual);
-		req->sense_len = virtio32_to_cpu(vblk->vdev, vbr->in_hdr.sense_len);
+		req->block_pc->sense_len =
+			virtio32_to_cpu(vblk->vdev, vbr->in_hdr.sense_len);
 		req->errors = virtio32_to_cpu(vblk->vdev, vbr->in_hdr.errors);
 	} else if (req->cmd_type == REQ_TYPE_DRV_PRIV) {
 		req->errors = (error != 0);
diff --git a/drivers/cdrom/cdrom.c b/drivers/cdrom/cdrom.c
index 5d28a45..7a5672a 100644
--- a/drivers/cdrom/cdrom.c
+++ b/drivers/cdrom/cdrom.c
@@ -2160,6 +2160,7 @@ static int cdrom_read_cdda_bpc(struct cdrom_device_info *cdi, __u8 __user *ubuf,
 			       int lba, int nframes)
 {
 	struct request_queue *q = cdi->disk->queue;
+	struct request_sense sense;
 	struct request *rq;
 	struct bio *bio;
 	unsigned int len;
@@ -2184,7 +2185,14 @@ static int cdrom_read_cdda_bpc(struct cdrom_device_info *cdi, __u8 __user *ubuf,
 			ret = PTR_ERR(rq);
 			break;
 		}
-		blk_rq_set_block_pc(rq);
+
+		memset(&sense, 0, sizeof(sense));
+
+		ret = blk_rq_set_block_pc(rq, 10, (u8 *)&sense, GFP_KERNEL);
+		if (ret) {
+			blk_put_request(rq);
+			break;
+		}
 
 		ret = blk_rq_map_user(q, rq, NULL, ubuf, len, GFP_KERNEL);
 		if (ret) {
@@ -2192,25 +2200,23 @@ static int cdrom_read_cdda_bpc(struct cdrom_device_info *cdi, __u8 __user *ubuf,
 			break;
 		}
 
-		rq->cmd[0] = GPCMD_READ_CD;
-		rq->cmd[1] = 1 << 2;
-		rq->cmd[2] = (lba >> 24) & 0xff;
-		rq->cmd[3] = (lba >> 16) & 0xff;
-		rq->cmd[4] = (lba >>  8) & 0xff;
-		rq->cmd[5] = lba & 0xff;
-		rq->cmd[6] = (nr >> 16) & 0xff;
-		rq->cmd[7] = (nr >>  8) & 0xff;
-		rq->cmd[8] = nr & 0xff;
-		rq->cmd[9] = 0xf8;
-
-		rq->cmd_len = 12;
+		rq->block_pc->cmd[0] = GPCMD_READ_CD;
+		rq->block_pc->cmd[1] = 1 << 2;
+		rq->block_pc->cmd[2] = (lba >> 24) & 0xff;
+		rq->block_pc->cmd[3] = (lba >> 16) & 0xff;
+		rq->block_pc->cmd[4] = (lba >>  8) & 0xff;
+		rq->block_pc->cmd[5] = lba & 0xff;
+		rq->block_pc->cmd[6] = (nr >> 16) & 0xff;
+		rq->block_pc->cmd[7] = (nr >>  8) & 0xff;
+		rq->block_pc->cmd[8] = nr & 0xff;
+		rq->block_pc->cmd[9] = 0xf8;
+
 		rq->timeout = 60 * HZ;
 		bio = rq->bio;
 
 		if (blk_execute_rq(q, cdi->disk, rq, 0)) {
-			struct request_sense *s = rq->sense;
 			ret = -EIO;
-			cdi->last_sense = s->sense_key;
+			cdi->last_sense = sense.sense_key;
 		}
 
 		if (blk_rq_unmap_user(bio))
diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index 8001fe9..17df896 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -1069,6 +1069,7 @@ static void dm_end_request(struct request *clone, int error)
 	struct mapped_device *md = tio->md;
 	struct request *rq = tio->orig;
 
+#if 0
 	if (rq->cmd_type == REQ_TYPE_BLOCK_PC) {
 		rq->errors = clone->errors;
 		rq->resid_len = clone->resid_len;
@@ -1081,6 +1082,7 @@ static void dm_end_request(struct request *clone, int error)
 			 */
 			rq->sense_len = clone->sense_len;
 	}
+#endif
 
 	free_rq_clone(clone);
 	blk_end_request_all(rq, error);
@@ -1773,9 +1775,11 @@ static int setup_clone(struct request *clone, struct request *rq,
 	if (r)
 		return r;
 
+#if 0
 	clone->cmd = rq->cmd;
 	clone->cmd_len = rq->cmd_len;
 	clone->sense = rq->sense;
+#endif
 	clone->end_io = end_clone_request;
 	clone->end_io_data = tio;
 
diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c
index 5bdaae1..34127ac 100644
--- a/drivers/message/fusion/mptsas.c
+++ b/drivers/message/fusion/mptsas.c
@@ -2321,8 +2321,8 @@ static int mptsas_smp_handler(struct Scsi_Host *shost, struct sas_rphy *rphy,
 		SmpPassthroughReply_t *smprep;
 
 		smprep = (SmpPassthroughReply_t *)ioc->sas_mgmt.reply;
-		memcpy(req->sense, smprep, sizeof(*smprep));
-		req->sense_len = sizeof(*smprep);
+		memcpy(req->block_pc->sense, smprep, sizeof(*smprep));
+		req->block_pc->sense_len = sizeof(*smprep);
 		req->resid_len = 0;
 		rsp->resid_len -= smprep->ResponseDataLength;
 	} else {
diff --git a/drivers/scsi/device_handler/scsi_dh_alua.c b/drivers/scsi/device_handler/scsi_dh_alua.c
index 854b568..9d2d781 100644
--- a/drivers/scsi/device_handler/scsi_dh_alua.c
+++ b/drivers/scsi/device_handler/scsi_dh_alua.c
@@ -107,7 +107,7 @@ static int realloc_buffer(struct alua_dh_data *h, unsigned len)
 }
 
 static struct request *get_alua_req(struct scsi_device *sdev,
-				    void *buffer, unsigned buflen, int rw)
+		struct alua_dh_data *h, unsigned buflen, int rw)
 {
 	struct request *rq;
 	struct request_queue *q = sdev->request_queue;
@@ -119,9 +119,16 @@ static struct request *get_alua_req(struct scsi_device *sdev,
 			    "%s: blk_get_request failed\n", __func__);
 		return NULL;
 	}
-	blk_rq_set_block_pc(rq);
 
-	if (buflen && blk_rq_map_kern(q, rq, buffer, buflen, GFP_NOIO)) {
+	memset(h->sense, 0, SCSI_SENSE_BUFFERSIZE);
+	h->senselen = 0;
+
+	if (blk_rq_set_block_pc(rq, 16, h->sense, GFP_NOIO)) {
+		blk_put_request(rq);
+		return NULL;
+	}
+
+	if (buflen && blk_rq_map_kern(q, rq, h->buff, buflen, GFP_NOIO)) {
 		blk_put_request(rq);
 		sdev_printk(KERN_INFO, sdev,
 			    "%s: blk_rq_map_kern failed\n", __func__);
@@ -145,27 +152,23 @@ static int submit_vpd_inquiry(struct scsi_device *sdev, struct alua_dh_data *h)
 	struct request *rq;
 	int err = SCSI_DH_RES_TEMP_UNAVAIL;
 
-	rq = get_alua_req(sdev, h->buff, h->bufflen, READ);
+	rq = get_alua_req(sdev, h, h->bufflen, READ);
 	if (!rq)
 		goto done;
 
 	/* Prepare the command. */
-	rq->cmd[0] = INQUIRY;
-	rq->cmd[1] = 1;
-	rq->cmd[2] = 0x83;
-	rq->cmd[4] = h->bufflen;
-	rq->cmd_len = COMMAND_SIZE(INQUIRY);
-
-	rq->sense = h->sense;
-	memset(rq->sense, 0, SCSI_SENSE_BUFFERSIZE);
-	rq->sense_len = h->senselen = 0;
+	rq->block_pc->cmd[0] = INQUIRY;
+	rq->block_pc->cmd[1] = 1;
+	rq->block_pc->cmd[2] = 0x83;
+	rq->block_pc->cmd[4] = h->bufflen;
+	rq->block_pc->cmd_len = COMMAND_SIZE(INQUIRY);
 
 	err = blk_execute_rq(rq->q, NULL, rq, 1);
 	if (err == -EIO) {
 		sdev_printk(KERN_INFO, sdev,
 			    "%s: evpd inquiry failed with %x\n",
 			    ALUA_DH_NAME, rq->errors);
-		h->senselen = rq->sense_len;
+		h->senselen = rq->block_pc->sense_len;
 		err = SCSI_DH_IO;
 	}
 	blk_put_request(rq);
@@ -183,32 +186,28 @@ static unsigned submit_rtpg(struct scsi_device *sdev, struct alua_dh_data *h,
 	struct request *rq;
 	int err = SCSI_DH_RES_TEMP_UNAVAIL;
 
-	rq = get_alua_req(sdev, h->buff, h->bufflen, READ);
+	rq = get_alua_req(sdev, h, h->bufflen, READ);
 	if (!rq)
 		goto done;
 
 	/* Prepare the command. */
-	rq->cmd[0] = MAINTENANCE_IN;
+	rq->block_pc->cmd[0] = MAINTENANCE_IN;
 	if (rtpg_ext_hdr_req)
-		rq->cmd[1] = MI_REPORT_TARGET_PGS | MI_EXT_HDR_PARAM_FMT;
+		rq->block_pc->cmd[1] = MI_REPORT_TARGET_PGS | MI_EXT_HDR_PARAM_FMT;
 	else
-		rq->cmd[1] = MI_REPORT_TARGET_PGS;
-	rq->cmd[6] = (h->bufflen >> 24) & 0xff;
-	rq->cmd[7] = (h->bufflen >> 16) & 0xff;
-	rq->cmd[8] = (h->bufflen >>  8) & 0xff;
-	rq->cmd[9] = h->bufflen & 0xff;
-	rq->cmd_len = COMMAND_SIZE(MAINTENANCE_IN);
-
-	rq->sense = h->sense;
-	memset(rq->sense, 0, SCSI_SENSE_BUFFERSIZE);
-	rq->sense_len = h->senselen = 0;
+		rq->block_pc->cmd[1] = MI_REPORT_TARGET_PGS;
+	rq->block_pc->cmd[6] = (h->bufflen >> 24) & 0xff;
+	rq->block_pc->cmd[7] = (h->bufflen >> 16) & 0xff;
+	rq->block_pc->cmd[8] = (h->bufflen >>  8) & 0xff;
+	rq->block_pc->cmd[9] = h->bufflen & 0xff;
+	rq->block_pc->cmd_len = COMMAND_SIZE(MAINTENANCE_IN);
 
 	err = blk_execute_rq(rq->q, NULL, rq, 1);
 	if (err == -EIO) {
 		sdev_printk(KERN_INFO, sdev,
 			    "%s: rtpg failed with %x\n",
 			    ALUA_DH_NAME, rq->errors);
-		h->senselen = rq->sense_len;
+		h->senselen = rq->block_pc->sense_len;
 		err = SCSI_DH_IO;
 	}
 	blk_put_request(rq);
@@ -237,7 +236,7 @@ static void stpg_endio(struct request *req, int error)
 		goto done;
 	}
 
-	if (req->sense_len > 0) {
+	if (req->block_pc->sense_len > 0) {
 		err = scsi_normalize_sense(h->sense, SCSI_SENSE_BUFFERSIZE,
 					   &sense_hdr);
 		if (!err) {
@@ -293,22 +292,19 @@ static unsigned submit_stpg(struct alua_dh_data *h)
 	h->buff[6] = (h->group_id >> 8) & 0xff;
 	h->buff[7] = h->group_id & 0xff;
 
-	rq = get_alua_req(sdev, h->buff, stpg_len, WRITE);
+	rq = get_alua_req(sdev, h, stpg_len, WRITE);
 	if (!rq)
 		return SCSI_DH_RES_TEMP_UNAVAIL;
 
 	/* Prepare the command. */
-	rq->cmd[0] = MAINTENANCE_OUT;
-	rq->cmd[1] = MO_SET_TARGET_PGS;
-	rq->cmd[6] = (stpg_len >> 24) & 0xff;
-	rq->cmd[7] = (stpg_len >> 16) & 0xff;
-	rq->cmd[8] = (stpg_len >>  8) & 0xff;
-	rq->cmd[9] = stpg_len & 0xff;
-	rq->cmd_len = COMMAND_SIZE(MAINTENANCE_OUT);
-
-	rq->sense = h->sense;
-	memset(rq->sense, 0, SCSI_SENSE_BUFFERSIZE);
-	rq->sense_len = h->senselen = 0;
+	rq->block_pc->cmd[0] = MAINTENANCE_OUT;
+	rq->block_pc->cmd[1] = MO_SET_TARGET_PGS;
+	rq->block_pc->cmd[6] = (stpg_len >> 24) & 0xff;
+	rq->block_pc->cmd[7] = (stpg_len >> 16) & 0xff;
+	rq->block_pc->cmd[8] = (stpg_len >>  8) & 0xff;
+	rq->block_pc->cmd[9] = stpg_len & 0xff;
+	rq->block_pc->cmd_len = COMMAND_SIZE(MAINTENANCE_OUT);
+
 	rq->end_io_data = h;
 
 	blk_execute_rq_nowait(rq->q, NULL, rq, 1, stpg_endio);
diff --git a/drivers/scsi/device_handler/scsi_dh_emc.c b/drivers/scsi/device_handler/scsi_dh_emc.c
index 6ed1caa..a009bf2 100644
--- a/drivers/scsi/device_handler/scsi_dh_emc.c
+++ b/drivers/scsi/device_handler/scsi_dh_emc.c
@@ -267,8 +267,8 @@ out:
  * Uses data and sense buffers in hardware handler context structure and
  * assumes serial servicing of commands, both issuance and completion.
  */
-static struct request *get_req(struct scsi_device *sdev, int cmd,
-				unsigned char *buffer)
+static struct request *get_req(struct scsi_device *sdev,
+		struct clariion_dh_data *csdev, int cmd)
 {
 	struct request *rq;
 	int len = 0;
@@ -280,25 +280,31 @@ static struct request *get_req(struct scsi_device *sdev, int cmd,
 		return NULL;
 	}
 
-	blk_rq_set_block_pc(rq);
-	rq->cmd_len = COMMAND_SIZE(cmd);
-	rq->cmd[0] = cmd;
+	memset(csdev->sense, 0, SCSI_SENSE_BUFFERSIZE);
+	csdev->senselen = 0;
+
+	if (blk_rq_set_block_pc(rq, COMMAND_SIZE(cmd),
+			csdev->sense, GFP_NOIO)) {
+		blk_put_request(rq);
+		return NULL;
+	}
+	rq->block_pc->cmd[0] = cmd;
 
 	switch (cmd) {
 	case MODE_SELECT:
 		len = sizeof(short_trespass);
-		rq->cmd[1] = 0x10;
-		rq->cmd[4] = len;
+		rq->block_pc->cmd[1] = 0x10;
+		rq->block_pc->cmd[4] = len;
 		break;
 	case MODE_SELECT_10:
 		len = sizeof(long_trespass);
-		rq->cmd[1] = 0x10;
-		rq->cmd[8] = len;
+		rq->block_pc->cmd[1] = 0x10;
+		rq->block_pc->cmd[8] = len;
 		break;
 	case INQUIRY:
 		len = CLARIION_BUFFER_SIZE;
-		rq->cmd[4] = len;
-		memset(buffer, 0, len);
+		rq->block_pc->cmd[4] = len;
+		memset(csdev->buffer, 0, len);
 		break;
 	default:
 		BUG_ON(1);
@@ -310,7 +316,7 @@ static struct request *get_req(struct scsi_device *sdev, int cmd,
 	rq->timeout = CLARIION_TIMEOUT;
 	rq->retries = CLARIION_RETRIES;
 
-	if (blk_rq_map_kern(rq->q, rq, buffer, len, GFP_NOIO)) {
+	if (blk_rq_map_kern(rq->q, rq, csdev->buffer, len, GFP_NOIO)) {
 		blk_put_request(rq);
 		return NULL;
 	}
@@ -321,20 +327,16 @@ static struct request *get_req(struct scsi_device *sdev, int cmd,
 static int send_inquiry_cmd(struct scsi_device *sdev, int page,
 			    struct clariion_dh_data *csdev)
 {
-	struct request *rq = get_req(sdev, INQUIRY, csdev->buffer);
+	struct request *rq = get_req(sdev, csdev, INQUIRY);
 	int err;
 
 	if (!rq)
 		return SCSI_DH_RES_TEMP_UNAVAIL;
 
-	rq->sense = csdev->sense;
-	memset(rq->sense, 0, SCSI_SENSE_BUFFERSIZE);
-	rq->sense_len = csdev->senselen = 0;
-
-	rq->cmd[0] = INQUIRY;
+	rq->block_pc->cmd[0] = INQUIRY;
 	if (page != 0) {
-		rq->cmd[1] = 1;
-		rq->cmd[2] = page;
+		rq->block_pc->cmd[1] = 1;
+		rq->block_pc->cmd[2] = page;
 	}
 	err = blk_execute_rq(sdev->request_queue, NULL, rq, 1);
 	if (err == -EIO) {
@@ -342,7 +344,7 @@ static int send_inquiry_cmd(struct scsi_device *sdev, int page,
 			    "%s: failed to send %s INQUIRY: %x\n",
 			    CLARIION_NAME, page?"EVPD":"standard",
 			    rq->errors);
-		csdev->senselen = rq->sense_len;
+		csdev->senselen = rq->block_pc->sense_len;
 		err = SCSI_DH_IO;
 	}
 
@@ -376,17 +378,13 @@ static int send_trespass_cmd(struct scsi_device *sdev,
 	BUG_ON((len > CLARIION_BUFFER_SIZE));
 	memcpy(csdev->buffer, page22, len);
 
-	rq = get_req(sdev, cmd, csdev->buffer);
+	rq = get_req(sdev, csdev, cmd);
 	if (!rq)
 		return SCSI_DH_RES_TEMP_UNAVAIL;
 
-	rq->sense = csdev->sense;
-	memset(rq->sense, 0, SCSI_SENSE_BUFFERSIZE);
-	rq->sense_len = csdev->senselen = 0;
-
 	err = blk_execute_rq(sdev->request_queue, NULL, rq, 1);
 	if (err == -EIO) {
-		if (rq->sense_len) {
+		if (rq->block_pc->sense_len) {
 			err = trespass_endio(sdev, csdev->sense);
 		} else {
 			sdev_printk(KERN_INFO, sdev,
diff --git a/drivers/scsi/device_handler/scsi_dh_hp_sw.c b/drivers/scsi/device_handler/scsi_dh_hp_sw.c
index 485d995..06f2c36 100644
--- a/drivers/scsi/device_handler/scsi_dh_hp_sw.c
+++ b/drivers/scsi/device_handler/scsi_dh_hp_sw.c
@@ -119,19 +119,21 @@ retry:
 	if (IS_ERR(req))
 		return SCSI_DH_RES_TEMP_UNAVAIL;
 
-	blk_rq_set_block_pc(req);
+	memset(h->sense, 0, SCSI_SENSE_BUFFERSIZE);
+
+	ret = blk_rq_set_block_pc(req, 16, h->sense, GFP_NOIO);
+	if (ret)
+		goto out;
+
 	req->cmd_flags |= REQ_FAILFAST_DEV | REQ_FAILFAST_TRANSPORT |
 			  REQ_FAILFAST_DRIVER;
-	req->cmd_len = COMMAND_SIZE(TEST_UNIT_READY);
-	req->cmd[0] = TEST_UNIT_READY;
+	req->block_pc->cmd_len = COMMAND_SIZE(TEST_UNIT_READY);
+	req->block_pc->cmd[0] = TEST_UNIT_READY;
 	req->timeout = HP_SW_TIMEOUT;
-	req->sense = h->sense;
-	memset(req->sense, 0, SCSI_SENSE_BUFFERSIZE);
-	req->sense_len = 0;
 
 	ret = blk_execute_rq(req->q, NULL, req, 1);
 	if (ret == -EIO) {
-		if (req->sense_len > 0) {
+		if (req->block_pc->sense_len > 0) {
 			ret = tur_done(sdev, h->sense);
 		} else {
 			sdev_printk(KERN_WARNING, sdev,
@@ -152,8 +154,8 @@ retry:
 		ret = SCSI_DH_OK;
 	}
 
+out:
 	blk_put_request(req);
-
 	return ret;
 }
 
@@ -212,7 +214,7 @@ static void start_stop_endio(struct request *req, int error)
 		goto done;
 	}
 
-	if (req->sense_len > 0) {
+	if (req->block_pc->sense_len > 0) {
 		err = start_done(h->sdev, h->sense);
 		if (err == SCSI_DH_RETRY) {
 			err = SCSI_DH_IO;
@@ -249,16 +251,20 @@ static int hp_sw_start_stop(struct hp_sw_dh_data *h)
 	if (IS_ERR(req))
 		return SCSI_DH_RES_TEMP_UNAVAIL;
 
-	blk_rq_set_block_pc(req);
+	memset(h->sense, 0, SCSI_SENSE_BUFFERSIZE);
+
+	if (blk_rq_set_block_pc(req, 16, h->sense, GFP_ATOMIC) < 0) {
+		blk_put_request(req);
+		return SCSI_DH_RES_TEMP_UNAVAIL;
+	}
+		
 	req->cmd_flags |= REQ_FAILFAST_DEV | REQ_FAILFAST_TRANSPORT |
 			  REQ_FAILFAST_DRIVER;
-	req->cmd_len = COMMAND_SIZE(START_STOP);
-	req->cmd[0] = START_STOP;
-	req->cmd[4] = 1;	/* Start spin cycle */
+	req->block_pc->cmd_len = COMMAND_SIZE(START_STOP);
+	req->block_pc->cmd[0] = START_STOP;
+	req->block_pc->cmd[4] = 1;	/* Start spin cycle */
 	req->timeout = HP_SW_TIMEOUT;
-	req->sense = h->sense;
-	memset(req->sense, 0, SCSI_SENSE_BUFFERSIZE);
-	req->sense_len = 0;
+
 	req->end_io_data = h;
 
 	blk_execute_rq_nowait(req->q, NULL, req, 1, start_stop_endio);
diff --git a/drivers/scsi/device_handler/scsi_dh_rdac.c b/drivers/scsi/device_handler/scsi_dh_rdac.c
index b46ace3..78552ae 100644
--- a/drivers/scsi/device_handler/scsi_dh_rdac.c
+++ b/drivers/scsi/device_handler/scsi_dh_rdac.c
@@ -266,7 +266,7 @@ static inline struct rdac_dh_data *get_rdac_data(struct scsi_device *sdev)
 }
 
 static struct request *get_rdac_req(struct scsi_device *sdev,
-			void *buffer, unsigned buflen, int rw)
+		struct rdac_dh_data *h, void *buffer, unsigned buflen, int rw)
 {
 	struct request *rq;
 	struct request_queue *q = sdev->request_queue;
@@ -278,7 +278,12 @@ static struct request *get_rdac_req(struct scsi_device *sdev,
 				"get_rdac_req: blk_get_request failed.\n");
 		return NULL;
 	}
-	blk_rq_set_block_pc(rq);
+
+	memset(h->sense, 0, SCSI_SENSE_BUFFERSIZE);
+	if (blk_rq_set_block_pc(rq, 16, h->sense, GFP_NOIO)) {
+		blk_put_request(rq);
+		return NULL;
+	}
 
 	if (buflen && blk_rq_map_kern(q, rq, buffer, buflen, GFP_NOIO)) {
 		blk_put_request(rq);
@@ -336,24 +341,20 @@ static struct request *rdac_failover_get(struct scsi_device *sdev,
 	}
 
 	/* get request for block layer packet command */
-	rq = get_rdac_req(sdev, &h->ctlr->mode_select, data_size, WRITE);
+	rq = get_rdac_req(sdev, h, &h->ctlr->mode_select, data_size, WRITE);
 	if (!rq)
 		return NULL;
 
 	/* Prepare the command. */
 	if (h->ctlr->use_ms10) {
-		rq->cmd[0] = MODE_SELECT_10;
-		rq->cmd[7] = data_size >> 8;
-		rq->cmd[8] = data_size & 0xff;
+		rq->block_pc->cmd[0] = MODE_SELECT_10;
+		rq->block_pc->cmd[7] = data_size >> 8;
+		rq->block_pc->cmd[8] = data_size & 0xff;
 	} else {
-		rq->cmd[0] = MODE_SELECT;
-		rq->cmd[4] = data_size;
+		rq->block_pc->cmd[0] = MODE_SELECT;
+		rq->block_pc->cmd[4] = data_size;
 	}
-	rq->cmd_len = COMMAND_SIZE(rq->cmd[0]);
-
-	rq->sense = h->sense;
-	memset(rq->sense, 0, SCSI_SENSE_BUFFERSIZE);
-	rq->sense_len = 0;
+	rq->block_pc->cmd_len = COMMAND_SIZE(rq->block_pc->cmd[0]);
 
 	return rq;
 }
@@ -409,20 +410,16 @@ static int submit_inquiry(struct scsi_device *sdev, int page_code,
 	struct request_queue *q = sdev->request_queue;
 	int err = SCSI_DH_RES_TEMP_UNAVAIL;
 
-	rq = get_rdac_req(sdev, &h->inq, len, READ);
+	rq = get_rdac_req(sdev, h, &h->inq, len, READ);
 	if (!rq)
 		goto done;
 
 	/* Prepare the command. */
-	rq->cmd[0] = INQUIRY;
-	rq->cmd[1] = 1;
-	rq->cmd[2] = page_code;
-	rq->cmd[4] = len;
-	rq->cmd_len = COMMAND_SIZE(INQUIRY);
-
-	rq->sense = h->sense;
-	memset(rq->sense, 0, SCSI_SENSE_BUFFERSIZE);
-	rq->sense_len = 0;
+	rq->block_pc->cmd[0] = INQUIRY;
+	rq->block_pc->cmd[1] = 1;
+	rq->block_pc->cmd[2] = page_code;
+	rq->block_pc->cmd[4] = len;
+	rq->block_pc->cmd_len = COMMAND_SIZE(INQUIRY);
 
 	err = blk_execute_rq(q, NULL, rq, 1);
 	if (err == -EIO)
diff --git a/drivers/scsi/mpt2sas/mpt2sas_transport.c b/drivers/scsi/mpt2sas/mpt2sas_transport.c
index ff2500a..672597c 100644
--- a/drivers/scsi/mpt2sas/mpt2sas_transport.c
+++ b/drivers/scsi/mpt2sas/mpt2sas_transport.c
@@ -2094,8 +2094,8 @@ _transport_smp_handler(struct Scsi_Host *shost, struct sas_rphy *rphy,
 		    ioc->name, __func__,
 		    le16_to_cpu(mpi_reply->ResponseDataLength)));
 
-		memcpy(req->sense, mpi_reply, sizeof(*mpi_reply));
-		req->sense_len = sizeof(*mpi_reply);
+		memcpy(req->block_pc->sense, mpi_reply, sizeof(*mpi_reply));
+		req->block_pc->sense_len = sizeof(*mpi_reply);
 		req->resid_len = 0;
 		rsp->resid_len -=
 		    le16_to_cpu(mpi_reply->ResponseDataLength);
diff --git a/drivers/scsi/mpt3sas/mpt3sas_transport.c b/drivers/scsi/mpt3sas/mpt3sas_transport.c
index efb98af..b558442 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_transport.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_transport.c
@@ -2054,8 +2054,8 @@ _transport_smp_handler(struct Scsi_Host *shost, struct sas_rphy *rphy,
 		    ioc->name, __func__,
 		    le16_to_cpu(mpi_reply->ResponseDataLength)));
 
-		memcpy(req->sense, mpi_reply, sizeof(*mpi_reply));
-		req->sense_len = sizeof(*mpi_reply);
+		memcpy(req->block_pc->sense, mpi_reply, sizeof(*mpi_reply));
+		req->block_pc->sense_len = sizeof(*mpi_reply);
 		req->resid_len = 0;
 		rsp->resid_len -=
 		    le16_to_cpu(mpi_reply->ResponseDataLength);
diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c
index 0cccd60..055714d 100644
--- a/drivers/scsi/osd/osd_initiator.c
+++ b/drivers/scsi/osd/osd_initiator.c
@@ -480,7 +480,7 @@ static void _set_error_resid(struct osd_request *or, struct request *req,
 {
 	or->async_error = error;
 	or->req_errors = req->errors ? : error;
-	or->sense_len = req->sense_len;
+	or->sense_len = req->block_pc->sense_len;
 	if (or->out.req)
 		or->out.residual = or->out.req->resid_len;
 	if (or->in.req)
@@ -1563,16 +1563,8 @@ static struct request *_make_request(struct request_queue *q, bool has_write,
 {
 	if (oii->bio)
 		return blk_make_request(q, oii->bio, flags);
-	else {
-		struct request *req;
-
-		req = blk_get_request(q, has_write ? WRITE : READ, flags);
-		if (IS_ERR(req))
-			return req;
-
-		blk_rq_set_block_pc(req);
-		return req;
-	}
+	else 
+		return blk_get_request(q, has_write ? WRITE : READ, flags);
 }
 
 static int _init_blk_request(struct osd_request *or,
@@ -1590,13 +1582,22 @@ static int _init_blk_request(struct osd_request *or,
 		goto out;
 	}
 
+	/* 
+	 * XXX: allocating max size here to avoid having to reorder all
+	 * the code below.
+	 */
+	ret = blk_rq_set_block_pc(req, 255, req->block_pc->sense, flags);
+	if (ret) {
+		blk_put_request(req);
+		goto out;
+	}
+
 	or->request = req;
 	req->cmd_flags |= REQ_QUIET;
 
 	req->timeout = or->timeout;
 	req->retries = or->retries;
-	req->sense = or->sense;
-	req->sense_len = 0;
+	req->block_pc->sense_len = 0;
 
 	if (has_out) {
 		or->out.req = req;
@@ -1608,7 +1609,6 @@ static int _init_blk_request(struct osd_request *or,
 				ret = PTR_ERR(req);
 				goto out;
 			}
-			blk_rq_set_block_pc(req);
 			or->in.req = or->request->next_rq = req;
 		}
 	} else if (has_in)
@@ -1695,8 +1695,8 @@ int osd_finalize_request(struct osd_request *or,
 
 	osd_sec_sign_cdb(&or->cdb, cap_key);
 
-	or->request->cmd = or->cdb.buff;
-	or->request->cmd_len = _osd_req_cdb_len(or);
+	memcpy(or->request->block_pc->cmd, or->cdb.buff, _osd_req_cdb_len(or));
+	or->request->block_pc->cmd_len = _osd_req_cdb_len(or);
 
 	return 0;
 }
diff --git a/drivers/scsi/osst.c b/drivers/scsi/osst.c
index 5033223..79b57fe 100644
--- a/drivers/scsi/osst.c
+++ b/drivers/scsi/osst.c
@@ -367,7 +367,9 @@ static int osst_execute(struct osst_request *SRpnt, const unsigned char *cmd,
 	if (IS_ERR(req))
 		return DRIVER_ERROR << 24;
 
-	blk_rq_set_block_pc(req);
+	err = blk_rq_set_block_pc(req, cmd_len, SRpnt->sense, GFP_KERNEL);
+	if (err)
+		goto free_req;
 	req->cmd_flags |= REQ_QUIET;
 
 	SRpnt->bio = NULL;
@@ -404,11 +406,7 @@ static int osst_execute(struct osst_request *SRpnt, const unsigned char *cmd,
 			goto free_req;
 	}
 
-	req->cmd_len = cmd_len;
-	memset(req->cmd, 0, BLK_MAX_CDB); /* ATAPI hates garbage after CDB */
-	memcpy(req->cmd, cmd, req->cmd_len);
-	req->sense = SRpnt->sense;
-	req->sense_len = 0;
+	memcpy(req->block_pc->cmd, cmd, cmd_len);
 	req->timeout = timeout;
 	req->retries = retries;
 	req->end_io_data = SRpnt;
diff --git a/drivers/scsi/qla2xxx/qla_bsg.c b/drivers/scsi/qla2xxx/qla_bsg.c
index 2e2bb6f..f6bf256 100644
--- a/drivers/scsi/qla2xxx/qla_bsg.c
+++ b/drivers/scsi/qla2xxx/qla_bsg.c
@@ -909,7 +909,7 @@ qla2x00_process_loopback(struct fc_bsg_job *bsg_job)
 
 	bsg_job->reply_len = sizeof(struct fc_bsg_reply) +
 	    sizeof(response) + sizeof(uint8_t);
-	fw_sts_ptr = ((uint8_t *)bsg_job->req->sense) +
+	fw_sts_ptr = ((uint8_t *)bsg_job->req->block_pc->sense) +
 	    sizeof(struct fc_bsg_reply);
 	memcpy(fw_sts_ptr, response, sizeof(response));
 	fw_sts_ptr += sizeof(response);
diff --git a/drivers/scsi/qla2xxx/qla_isr.c b/drivers/scsi/qla2xxx/qla_isr.c
index 6dc14cd..724eaa4 100644
--- a/drivers/scsi/qla2xxx/qla_isr.c
+++ b/drivers/scsi/qla2xxx/qla_isr.c
@@ -1418,7 +1418,8 @@ qla24xx_els_ct_entry(scsi_qla_host_t *vha, struct req_que *req,
 			    type, sp->handle, comp_status, fw_status[1], fw_status[2],
 			    le16_to_cpu(((struct els_sts_entry_24xx *)
 				pkt)->total_byte_count));
-			fw_sts_ptr = ((uint8_t*)bsg_job->req->sense) + sizeof(struct fc_bsg_reply);
+			fw_sts_ptr = ((uint8_t*)bsg_job->req->block_pc->sense) +
+					sizeof(struct fc_bsg_reply);
 			memcpy( fw_sts_ptr, fw_status, sizeof(fw_status));
 		}
 		else {
@@ -1432,7 +1433,8 @@ qla24xx_els_ct_entry(scsi_qla_host_t *vha, struct req_que *req,
 				    pkt)->error_subcode_2));
 			res = DID_ERROR << 16;
 			bsg_job->reply->reply_payload_rcv_len = 0;
-			fw_sts_ptr = ((uint8_t*)bsg_job->req->sense) + sizeof(struct fc_bsg_reply);
+			fw_sts_ptr = ((uint8_t*)bsg_job->req->block_pc->sense) +
+					sizeof(struct fc_bsg_reply);
 			memcpy( fw_sts_ptr, fw_status, sizeof(fw_status));
 		}
 		ql_dump_buffer(ql_dbg_user + ql_dbg_buffer, vha, 0x5056,
diff --git a/drivers/scsi/qla2xxx/qla_mr.c b/drivers/scsi/qla2xxx/qla_mr.c
index 6d190b4..2ac1886 100644
--- a/drivers/scsi/qla2xxx/qla_mr.c
+++ b/drivers/scsi/qla2xxx/qla_mr.c
@@ -2242,7 +2242,7 @@ qlafx00_ioctl_iosb_entry(scsi_qla_host_t *vha, struct req_que *req,
 		memcpy(fstatus.reserved_3,
 		    pkt->reserved_2, 20 * sizeof(uint8_t));
 
-		fw_sts_ptr = ((uint8_t *)bsg_job->req->sense) +
+		fw_sts_ptr = ((uint8_t *)bsg_job->req->block_pc->sense) +
 		    sizeof(struct fc_bsg_reply);
 
 		memcpy(fw_sts_ptr, (uint8_t *)&fstatus,
diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index c95a4e9..85d659e 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -1968,16 +1968,18 @@ static void scsi_eh_lock_door(struct scsi_device *sdev)
 	if (IS_ERR(req))
 		return;
 
-	blk_rq_set_block_pc(req);
-
-	req->cmd[0] = ALLOW_MEDIUM_REMOVAL;
-	req->cmd[1] = 0;
-	req->cmd[2] = 0;
-	req->cmd[3] = 0;
-	req->cmd[4] = SCSI_REMOVAL_PREVENT;
-	req->cmd[5] = 0;
+	if (blk_rq_set_block_pc(req, COMMAND_SIZE(ALLOW_MEDIUM_REMOVAL),
+				NULL, GFP_KERNEL) < 0) {
+		blk_put_request(req);
+		return;
+	}
 
-	req->cmd_len = COMMAND_SIZE(req->cmd[0]);
+	req->block_pc->cmd[0] = ALLOW_MEDIUM_REMOVAL;
+	req->block_pc->cmd[1] = 0;
+	req->block_pc->cmd[2] = 0;
+	req->block_pc->cmd[3] = 0;
+	req->block_pc->cmd[4] = SCSI_REMOVAL_PREVENT;
+	req->block_pc->cmd[5] = 0;
 
 	req->cmd_flags |= REQ_QUIET;
 	req->timeout = 10 * HZ;
@@ -2332,8 +2334,6 @@ scsi_ioctl_reset(struct scsi_device *dev, int __user *arg)
 	blk_rq_init(NULL, &req);
 	scmd->request = &req;
 
-	scmd->cmnd = req.cmd;
-
 	scmd->scsi_done		= scsi_reset_provider_done_command;
 	memset(&scmd->sdb, 0, sizeof(scmd->sdb));
 
diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index b1a2631..29e0cd6 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -224,16 +224,15 @@ int scsi_execute(struct scsi_device *sdev, const unsigned char *cmd,
 	req = blk_get_request(sdev->request_queue, write, __GFP_WAIT);
 	if (IS_ERR(req))
 		return ret;
-	blk_rq_set_block_pc(req);
+
+	if (blk_rq_set_block_pc(req, COMMAND_SIZE(cmd[0]), sense, GFP_KERNEL))
+		goto out;
 
 	if (bufflen &&	blk_rq_map_kern(sdev->request_queue, req,
 					buffer, bufflen, __GFP_WAIT))
 		goto out;
 
-	req->cmd_len = COMMAND_SIZE(cmd[0]);
-	memcpy(req->cmd, cmd, req->cmd_len);
-	req->sense = sense;
-	req->sense_len = 0;
+	memcpy(req->block_pc->cmd, cmd, req->block_pc->cmd_len);
 	req->retries = retries;
 	req->timeout = timeout;
 	req->cmd_flags |= flags | REQ_QUIET | REQ_PREEMPT;
@@ -835,7 +834,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 
 	if (req->cmd_type == REQ_TYPE_BLOCK_PC) { /* SG_IO ioctl from block level */
 		if (result) {
-			if (sense_valid && req->sense) {
+			if (sense_valid && req->block_pc->sense) {
 				/*
 				 * SG_IO wants current and deferred errors
 				 */
@@ -843,8 +842,8 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes)
 
 				if (len > SCSI_SENSE_BUFFERSIZE)
 					len = SCSI_SENSE_BUFFERSIZE;
-				memcpy(req->sense, cmd->sense_buffer,  len);
-				req->sense_len = len;
+				memcpy(req->block_pc->sense, cmd->sense_buffer,  len);
+				req->block_pc->sense_len = len;
 			}
 			if (!sense_deferred)
 				error = __scsi_error_from_host_byte(cmd, result);
@@ -1208,7 +1207,6 @@ static struct scsi_cmnd *scsi_get_cmd_from_req(struct scsi_device *sdev,
 	cmd->tag = req->tag;
 	cmd->request = req;
 
-	cmd->cmnd = req->cmd;
 	cmd->prot_op = SCSI_PROT_NORMAL;
 
 	return cmd;
@@ -1234,7 +1232,8 @@ static int scsi_setup_blk_pc_cmnd(struct scsi_device *sdev, struct request *req)
 		memset(&cmd->sdb, 0, sizeof(cmd->sdb));
 	}
 
-	cmd->cmd_len = req->cmd_len;
+	cmd->cmd_len = req->block_pc->cmd_len;
+	cmd->cmnd = req->block_pc->cmd;
 	cmd->transfersize = blk_rq_bytes(req);
 	cmd->allowed = req->retries;
 	return BLKPREP_OK;
@@ -1255,7 +1254,7 @@ static int scsi_setup_fs_cmnd(struct scsi_device *sdev, struct request *req)
 			return ret;
 	}
 
-	memset(cmd->cmnd, 0, BLK_MAX_CDB);
+	cmd->cmnd = cmd->__cmnd;
 	return scsi_cmd_to_driver(cmd)->init_command(cmd);
 }
 
@@ -1911,7 +1910,6 @@ static int scsi_mq_prep_fn(struct request *req)
 
 	cmd->tag = req->tag;
 
-	cmd->cmnd = req->cmd;
 	cmd->prot_op = SCSI_PROT_NORMAL;
 
 	INIT_LIST_HEAD(&cmd->list);
diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index 24eaaf6..6024e6c 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -3593,9 +3593,9 @@ fc_bsg_jobdone(struct fc_bsg_job *job)
 
 	if (err < 0)
 		/* we're only returning the result field in the reply */
-		job->req->sense_len = sizeof(uint32_t);
+		job->req->block_pc->sense_len = sizeof(uint32_t);
 	else
-		job->req->sense_len = job->reply_len;
+		job->req->block_pc->sense_len = job->reply_len;
 
 	/* we assume all request payload was transferred, residual == 0 */
 	req->resid_len = 0;
@@ -3725,9 +3725,9 @@ fc_req_to_bsgjob(struct Scsi_Host *shost, struct fc_rport *rport,
 	if (i->f->dd_bsg_size)
 		job->dd_data = (void *)&job[1];
 	spin_lock_init(&job->job_lock);
-	job->request = (struct fc_bsg_request *)req->cmd;
-	job->request_len = req->cmd_len;
-	job->reply = req->sense;
+	job->request = (struct fc_bsg_request *)req->block_pc->cmd;
+	job->request_len = req->block_pc->cmd_len;
+	job->reply = req->block_pc->sense;
 	job->reply_len = SCSI_SENSE_BUFFERSIZE;	/* Size of sense buffer
 						 * allocated */
 	if (req->bio) {
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index dcc4244..7883fd9 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -126,9 +126,6 @@ static DEFINE_IDA(sd_index_ida);
  * object after last put) */
 static DEFINE_MUTEX(sd_ref_mutex);
 
-static struct kmem_cache *sd_cdb_cache;
-static mempool_t *sd_cdb_pool;
-
 static const char *sd_cache_types[] = {
 	"write through", "none", "write back",
 	"write back, no read (daft)"
@@ -1016,13 +1013,6 @@ static int sd_setup_read_write_cmnd(struct scsi_cmnd *SCpnt)
 		protect = 0;
 
 	if (protect && sdkp->protection_type == SD_DIF_TYPE2_PROTECTION) {
-		SCpnt->cmnd = mempool_alloc(sd_cdb_pool, GFP_ATOMIC);
-
-		if (unlikely(SCpnt->cmnd == NULL)) {
-			ret = BLKPREP_DEFER;
-			goto out;
-		}
-
 		SCpnt->cmd_len = SD_EXT_CDB_SIZE;
 		memset(SCpnt->cmnd, 0, SCpnt->cmd_len);
 		SCpnt->cmnd[0] = VARIABLE_LENGTH_CMD;
@@ -1138,12 +1128,6 @@ static void sd_uninit_command(struct scsi_cmnd *SCpnt)
 
 	if (rq->cmd_flags & REQ_DISCARD)
 		__free_page(rq->completion_data);
-
-	if (SCpnt->cmnd != rq->cmd) {
-		mempool_free(SCpnt->cmnd, sd_cdb_pool);
-		SCpnt->cmnd = NULL;
-		SCpnt->cmd_len = 0;
-	}
 }
 
 /**
@@ -3221,38 +3205,17 @@ static int __init init_sd(void)
 
 	err = class_register(&sd_disk_class);
 	if (err)
-		goto err_out;
-
-	sd_cdb_cache = kmem_cache_create("sd_ext_cdb", SD_EXT_CDB_SIZE,
-					 0, 0, NULL);
-	if (!sd_cdb_cache) {
-		printk(KERN_ERR "sd: can't init extended cdb cache\n");
-		err = -ENOMEM;
-		goto err_out_class;
-	}
-
-	sd_cdb_pool = mempool_create_slab_pool(SD_MEMPOOL_SIZE, sd_cdb_cache);
-	if (!sd_cdb_pool) {
-		printk(KERN_ERR "sd: can't init extended cdb pool\n");
-		err = -ENOMEM;
-		goto err_out_cache;
-	}
+		goto out_unregister_blkdev;
 
 	err = scsi_register_driver(&sd_template.gendrv);
 	if (err)
-		goto err_out_driver;
+		goto out_unregister_class;
 
 	return 0;
 
-err_out_driver:
-	mempool_destroy(sd_cdb_pool);
-
-err_out_cache:
-	kmem_cache_destroy(sd_cdb_cache);
-
-err_out_class:
+out_unregister_class:
 	class_unregister(&sd_disk_class);
-err_out:
+out_unregister_blkdev:
 	for (i = 0; i < SD_MAJORS; i++)
 		unregister_blkdev(sd_major(i), "sd");
 	return err;
@@ -3270,8 +3233,6 @@ static void __exit exit_sd(void)
 	SCSI_LOG_HLQUEUE(3, printk("exit_sd: exiting sd driver\n"));
 
 	scsi_unregister_driver(&sd_template.gendrv);
-	mempool_destroy(sd_cdb_pool);
-	kmem_cache_destroy(sd_cdb_cache);
 
 	class_unregister(&sd_disk_class);
 
diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 9d7b7db..a266319 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -1297,7 +1297,7 @@ sg_rq_end_io(struct request *rq, int uptodate)
 	if (unlikely(atomic_read(&sdp->detaching)))
 		pr_info("%s: device detaching\n", __func__);
 
-	sense = rq->sense;
+	sense = rq->block_pc->sense;
 	result = rq->errors;
 	resid = rq->resid_len;
 
@@ -1342,8 +1342,6 @@ sg_rq_end_io(struct request *rq, int uptodate)
 	 * blk_rq_unmap_user() can be called from user context.
 	 */
 	srp->rq = NULL;
-	if (rq->cmd != rq->__cmd)
-		kfree(rq->cmd);
 	__blk_put_request(rq->q, rq);
 
 	write_lock_irqsave(&sfp->rq_list_lock, iflags);
@@ -1701,16 +1699,16 @@ sg_start_req(Sg_request *srp, unsigned char *cmd)
 		return PTR_ERR(rq);
 	}
 
-	blk_rq_set_block_pc(rq);
+	res = blk_rq_set_block_pc(rq, hp->cmd_len, srp->sense_b, GFP_KERNEL);
+	if (res) {
+		blk_put_request(rq);
+		return res;
+	}
 
-	if (hp->cmd_len > BLK_MAX_CDB)
-		rq->cmd = long_cmdp;
-	memcpy(rq->cmd, cmd, hp->cmd_len);
-	rq->cmd_len = hp->cmd_len;
+	memcpy(rq->block_pc->cmd, cmd, hp->cmd_len);
 
 	srp->rq = rq;
 	rq->end_io_data = srp;
-	rq->sense = srp->sense_b;
 	rq->retries = SG_DEFAULT_RETRIES;
 
 	if ((dxfer_len <= 0) || (dxfer_dir == SG_DXFER_NONE))
@@ -1785,11 +1783,8 @@ sg_finish_rem_req(Sg_request *srp)
 	if (srp->bio)
 		ret = blk_rq_unmap_user(srp->bio);
 
-	if (srp->rq) {
-		if (srp->rq->cmd != srp->rq->__cmd)
-			kfree(srp->rq->cmd);
+	if (srp->rq)
 		blk_put_request(srp->rq);
-	}
 
 	if (srp->res_used)
 		sg_unlink_reserve(sfp, srp);
diff --git a/drivers/scsi/st.c b/drivers/scsi/st.c
index 9a1c342..7b4445f 100644
--- a/drivers/scsi/st.c
+++ b/drivers/scsi/st.c
@@ -502,7 +502,10 @@ static int st_scsi_execute(struct st_request *SRpnt, const unsigned char *cmd,
 	if (IS_ERR(req))
 		return DRIVER_ERROR << 24;
 
-	blk_rq_set_block_pc(req);
+	err = blk_rq_set_block_pc(req, COMMAND_SIZE(cmd[0]), SRpnt->sense,
+			GFP_KERNEL);
+	if (err)
+		goto out_put_request;
 	req->cmd_flags |= REQ_QUIET;
 
 	mdata->null_mapped = 1;
@@ -510,24 +513,22 @@ static int st_scsi_execute(struct st_request *SRpnt, const unsigned char *cmd,
 	if (bufflen) {
 		err = blk_rq_map_user(req->q, req, mdata, NULL, bufflen,
 				      GFP_KERNEL);
-		if (err) {
-			blk_put_request(req);
-			return DRIVER_ERROR << 24;
-		}
+		if (err)
+			goto out_put_request;
 	}
 
 	SRpnt->bio = req->bio;
-	req->cmd_len = COMMAND_SIZE(cmd[0]);
-	memset(req->cmd, 0, BLK_MAX_CDB);
-	memcpy(req->cmd, cmd, req->cmd_len);
-	req->sense = SRpnt->sense;
-	req->sense_len = 0;
+	memcpy(req->block_pc->cmd, cmd, req->block_pc->cmd_len);
 	req->timeout = timeout;
 	req->retries = retries;
 	req->end_io_data = SRpnt;
 
 	blk_execute_rq_nowait(req->q, NULL, req, 1, st_scsi_execute_end);
 	return 0;
+
+out_put_request:
+	blk_put_request(req);
+	return DRIVER_ERROR << 24;
 }
 
 /* Do the scsi command. Waits until command performed if do_wait is true.
diff --git a/drivers/target/target_core_pscsi.c b/drivers/target/target_core_pscsi.c
index f6c954c..023accd 100644
--- a/drivers/target/target_core_pscsi.c
+++ b/drivers/target/target_core_pscsi.c
@@ -1064,8 +1064,6 @@ pscsi_execute_cmd(struct se_cmd *cmd)
 			ret = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
 			goto fail;
 		}
-
-		blk_rq_set_block_pc(req);
 	} else {
 		BUG_ON(!cmd->data_length);
 
@@ -1082,12 +1080,16 @@ pscsi_execute_cmd(struct se_cmd *cmd)
 		}
 	}
 
+	if (blk_rq_set_block_pc(req, scsi_command_size(pt->pscsi_cdb),
+			&pt->pscsi_sense[0], GFP_KERNEL)) {
+		blk_put_request(req);
+		ret = TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE;
+		goto fail;
+	}
+
 	req->end_io = pscsi_req_done;
 	req->end_io_data = cmd;
-	req->cmd_len = scsi_command_size(pt->pscsi_cdb);
-	req->cmd = &pt->pscsi_cdb[0];
-	req->sense = &pt->pscsi_sense[0];
-	req->sense_len = 0;
+	memcpy(req->block_pc->cmd, &pt->pscsi_cdb[0], req->block_pc->cmd_len);
 	if (pdv->pdv_sd->type == TYPE_DISK)
 		req->timeout = PS_TIMEOUT_DISK;
 	else
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 2da818a..e64a01b 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -80,6 +80,16 @@ enum rq_cmd_type_bits {
 #define BLK_MAX_CDB	16
 
 /*
+ * when request is used as a packet command carrier
+ */
+struct block_pc_request {
+	unsigned short cmd_len;
+	unsigned int sense_len;
+	void *sense;
+	unsigned char cmd[];
+};
+
+/*
  * Try to put the fields that are referenced together in the same cacheline.
  *
  * If you modify this structure, make sure to update blk_rq_init() and
@@ -172,23 +182,19 @@ struct request {
 	int tag;
 	int errors;
 
-	/*
-	 * when request is used as a packet command carrier
-	 */
-	unsigned char __cmd[BLK_MAX_CDB];
-	unsigned char *cmd;
-	unsigned short cmd_len;
-
 	unsigned int extra_len;	/* length of alignment and padding */
-	unsigned int sense_len;
 	unsigned int resid_len;	/* residual count */
-	void *sense;
 
 	unsigned long deadline;
 	struct list_head timeout_list;
 	unsigned int timeout;
 	int retries;
 
+	union {
+		struct block_pc_request *block_pc;
+		void *drv_private;
+	};
+
 	/*
 	 * completion callback.
 	 */
@@ -769,7 +775,8 @@ extern void __blk_put_request(struct request_queue *, struct request *);
 extern struct request *blk_get_request(struct request_queue *, int, gfp_t);
 extern struct request *blk_make_request(struct request_queue *, struct bio *,
 					gfp_t);
-extern void blk_rq_set_block_pc(struct request *);
+int blk_rq_set_block_pc(struct request *rq, unsigned short cmd_len,
+		u8 *sense, gfp_t gfp);
 extern void blk_requeue_request(struct request_queue *, struct request *);
 extern void blk_add_request_payload(struct request *rq, struct page *page,
 		unsigned int len);
diff --git a/include/linux/blktrace_api.h b/include/linux/blktrace_api.h
index afc1343..8c63eca 100644
--- a/include/linux/blktrace_api.h
+++ b/include/linux/blktrace_api.h
@@ -105,7 +105,9 @@ struct compat_blk_user_trace_setup {
 
 static inline int blk_cmd_buf_len(struct request *rq)
 {
-	return (rq->cmd_type == REQ_TYPE_BLOCK_PC) ? rq->cmd_len * 3 : 1;
+	if (rq->cmd_type == REQ_TYPE_BLOCK_PC)
+		return rq->block_pc->cmd_len * 3;
+	return 1;
 }
 
 extern void blk_dump_cmd(char *buf, struct request *rq);
diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 9fc1aec..de6d7d0 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -92,7 +92,7 @@ struct scsi_cmnd {
 
 	/* These elements define the operation we are about to perform */
 	unsigned char *cmnd;
-
+	unsigned char __cmnd[32];
 
 	/* These elements define the operation we ultimately want to perform */
 	struct scsi_data_buffer sdb;
diff --git a/kernel/trace/blktrace.c b/kernel/trace/blktrace.c
index 483cecf..e87c0ac 100644
--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c
@@ -720,7 +720,9 @@ static void blk_add_trace_rq(struct request_queue *q, struct request *rq,
 	if (rq->cmd_type == REQ_TYPE_BLOCK_PC) {
 		what |= BLK_TC_ACT(BLK_TC_PC);
 		__blk_add_trace(bt, 0, nr_bytes, rq->cmd_flags,
-				what, rq->errors, rq->cmd_len, rq->cmd);
+				what, rq->errors,
+				rq->block_pc->cmd_len,
+				rq->block_pc->cmd);
 	} else  {
 		what |= BLK_TC_ACT(BLK_TC_FS);
 		__blk_add_trace(bt, blk_rq_pos(rq), nr_bytes,
@@ -1762,14 +1764,17 @@ void blk_trace_remove_sysfs(struct device *dev)
 void blk_dump_cmd(char *buf, struct request *rq)
 {
 	int i, end;
-	int len = rq->cmd_len;
-	unsigned char *cmd = rq->cmd;
+	int len;
+	unsigned char *cmd;
 
 	if (rq->cmd_type != REQ_TYPE_BLOCK_PC) {
 		buf[0] = '\0';
 		return;
 	}
 
+	len = rq->block_pc->cmd_len;
+	cmd = rq->block_pc->cmd;
+
 	for (end = len - 1; end >= 0; end--)
 		if (cmd[end])
 			break;
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: RFC: struct request cleanups
  2015-04-17 20:37 RFC: struct request cleanups Christoph Hellwig
                   ` (6 preceding siblings ...)
  2015-04-17 20:37 ` [PATCH 7/7] block: allocate block_pc data separate from struct request Christoph Hellwig
@ 2015-04-21 19:51 ` Matthew Wilcox
  2015-05-05 19:41 ` Jens Axboe
  8 siblings, 0 replies; 12+ messages in thread
From: Matthew Wilcox @ 2015-04-21 19:51 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: axboe, linux-nvme, linux-scsi

On Fri, Apr 17, 2015 at 10:37:15PM +0200, Christoph Hellwig wrote:
> The real RFC is the last one which allocates the block_pc specific
> data separately in the callers instead of bloating every struct
> request with it.  I always hated what we did, but with the upcoming
> split of nvme into transports and command sets we'll need a NVME
> equivalent of BLOCK_PC, and as NVMe was designed by crackmonkeys
> dreaming of an ATA controller the "command block" for NVME is even
> bigger than what we have to deal with in SCSI.

Umm, the NVMe command was never supposed to be "transported".  The
crackmonkeys are the ones who are trying to ship it across the network
instead of designing a proper network protocol.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: RFC: struct request cleanups
  2015-04-17 20:37 RFC: struct request cleanups Christoph Hellwig
                   ` (7 preceding siblings ...)
  2015-04-21 19:51 ` RFC: struct request cleanups Matthew Wilcox
@ 2015-05-05 19:41 ` Jens Axboe
  8 siblings, 0 replies; 12+ messages in thread
From: Jens Axboe @ 2015-05-05 19:41 UTC (permalink / raw)
  To: Christoph Hellwig, linux-nvme; +Cc: linux-scsi

On 04/17/2015 02:37 PM, Christoph Hellwig wrote:
> The first 5 patches move the magic IDE request types into the old IDE
> driver to keep the core block code clean of them.  Those are basically
> ready to merge, just like the 6th one which is a cleanup on it's own.
>
> The real RFC is the last one which allocates the block_pc specific
> data separately in the callers instead of bloating every struct
> request with it.  I always hated what we did, but with the upcoming
> split of nvme into transports and command sets we'll need a NVME
> equivalent of BLOCK_PC, and as NVMe was designed by crackmonkeys
> dreaming of an ATA controller the "command block" for NVME is even
> bigger than what we have to deal with in SCSI.
>
> Note that the old IDE driver doesn't compile with the last patch
> yet as there are major nightmares to sort out, and BLOCK_PC passthrough
> with dm-multipath doesn't work yet either.  If I get some general
> concensus on the approach I'll fix those of course.

These are nice improvements, it'd be great to get rid of embeeding the 
command block. I have applied 1-6 for 4.2.

-- 
Jens Axboe


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 7/7] block: allocate block_pc data separate from struct request
  2015-04-17 20:37 ` [PATCH 7/7] block: allocate block_pc data separate from struct request Christoph Hellwig
@ 2015-05-06 11:46   ` Boaz Harrosh
  2015-05-11  8:00     ` Christoph Hellwig
  0 siblings, 1 reply; 12+ messages in thread
From: Boaz Harrosh @ 2015-05-06 11:46 UTC (permalink / raw)
  To: Christoph Hellwig, axboe, linux-nvme; +Cc: linux-scsi

On 04/17/2015 11:37 PM, Christoph Hellwig wrote:
> Don't bloat struct request with BLOCK_PC specific fields.
> 
> WIP, breaks dm BLOCK_PC passthrough and the old IDE driver for now.
<>
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 2e5020f..5d78a85 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
<>
> @@ -1274,15 +1270,24 @@ EXPORT_SYMBOL(blk_make_request);
>  /**
>   * blk_rq_set_block_pc - initialize a request to type BLOCK_PC
>   * @rq:		request to be initialized
> + * @cmd_len:	length of the CDB
> + * @gfp:	kmalloc flags
>   *
>   */
> -void blk_rq_set_block_pc(struct request *rq)
> +int blk_rq_set_block_pc(struct request *rq, unsigned short cmd_len,
> +		u8 *sense, gfp_t gfp)
>  {
>  	rq->cmd_type = REQ_TYPE_BLOCK_PC;
>  	rq->__data_len = 0;
>  	rq->__sector = (sector_t) -1;
>  	rq->bio = rq->biotail = NULL;
> -	memset(rq->__cmd, 0, sizeof(rq->__cmd));
> +
> +	rq->block_pc = kzalloc(sizeof(*rq->block_pc) + cmd_len, gfp);

I wish you would not embed a dynamic allocation here for any
driver regardless. This extra allocation does hurt a lot. See how in
SCSI fs commands you embedded it in scsi_cmnd so it catches as well
for multi-Q pre-allocation.

I think you need to just make it the same as *sense pass it from outside
and the allocation is the caller responsibility. The caller must have
an end-request call back set. (Or is a sync call)

Usually all users already have a structure to put this in. The only bit
more work is to take care of the free. If they are already passing sense
then they already need to free at end of request. Those that are synchronous
can have it on the stack. Only very few places need a bit of extra work.


> +	if (!rq->block_pc)
> +		return -ENOMEM;
> +	rq->block_pc->cmd_len = cmd_len;
> +	rq->block_pc->sense = sense;
> +	return 0;
>  }
>  EXPORT_SYMBOL(blk_rq_set_block_pc);
>  
> @@ -1379,6 +1384,10 @@ void __blk_put_request(struct request_queue *q, struct request *req)
>  	if (unlikely(!q))
>  		return;
>  
> +	/* could also be other type-specific data */
> +	if (req->block_pc)
> +		kfree(req->block_pc);
> +
>  	if (q->mq_ops) {
>  		blk_mq_free_request(req);
>  		return;
> diff --git a/block/blk-exec.c b/block/blk-exec.c
> index 3fec8a2..94e909e 100644
> --- a/block/blk-exec.c
> +++ b/block/blk-exec.c
> @@ -10,11 +10,6 @@
>  
>  #include "blk.h"
>  
> -/*
> - * for max sense size
> - */
> -#include <scsi/scsi_cmnd.h>
> -
>  /**
>   * blk_end_sync_rq - executes a completion event on a request
>   * @rq: request to complete
> @@ -100,16 +95,9 @@ int blk_execute_rq(struct request_queue *q, struct gendisk *bd_disk,
>  		   struct request *rq, int at_head)
>  {
>  	DECLARE_COMPLETION_ONSTACK(wait);
> -	char sense[SCSI_SENSE_BUFFERSIZE];
>  	int err = 0;
>  	unsigned long hang_check;
>  
> -	if (!rq->sense) {
> -		memset(sense, 0, sizeof(sense));
> -		rq->sense = sense;
> -		rq->sense_len = 0;
> -	}
> -
>  	rq->end_io_data = &wait;
>  	blk_execute_rq_nowait(q, bd_disk, rq, at_head, blk_end_sync_rq);
>  
> @@ -123,11 +111,6 @@ int blk_execute_rq(struct request_queue *q, struct gendisk *bd_disk,
>  	if (rq->errors)
>  		err = -EIO;
>  
> -	if (rq->sense == sense)	{
> -		rq->sense = NULL;
> -		rq->sense_len = 0;
> -	}
> -
>  	return err;
>  }
>  EXPORT_SYMBOL(blk_execute_rq);
<>
> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> index 2da818a..e64a01b 100644
> --- a/include/linux/blkdev.h
> +++ b/include/linux/blkdev.h
> @@ -80,6 +80,16 @@ enum rq_cmd_type_bits {
>  #define BLK_MAX_CDB	16
>  
>  /*
> + * when request is used as a packet command carrier
> + */
> +struct block_pc_request {
> +	unsigned short cmd_len;
> +	unsigned int sense_len;
> +	void *sense;

Better move cmd_len here the short will combine well
with the char array.

> +	unsigned char cmd[];
> +};
> +
> +/*
>   * Try to put the fields that are referenced together in the same cacheline.
>   *
>   * If you modify this structure, make sure to update blk_rq_init() and
> @@ -172,23 +182,19 @@ struct request {
>  	int tag;
>  	int errors;
>  
> -	/*
> -	 * when request is used as a packet command carrier
> -	 */
> -	unsigned char __cmd[BLK_MAX_CDB];
> -	unsigned char *cmd;
> -	unsigned short cmd_len;
> -
>  	unsigned int extra_len;	/* length of alignment and padding */
> -	unsigned int sense_len;
>  	unsigned int resid_len;	/* residual count */
> -	void *sense;
>  
>  	unsigned long deadline;
>  	struct list_head timeout_list;
>  	unsigned int timeout;
>  	int retries;
>  
> +	union {
> +		struct block_pc_request *block_pc;
> +		void *drv_private;
> +	};
> +

Exactly. drv_private is allocated by caller we can do the same for
block_pc. Also If (theoretically) a driver needs both it is just the
same pointer right? (container_of)

>  	/*
>  	 * completion callback.
>  	 */
> @@ -769,7 +775,8 @@ extern void __blk_put_request(struct request_queue *, struct request *);
>  extern struct request *blk_get_request(struct request_queue *, int, gfp_t);
>  extern struct request *blk_make_request(struct request_queue *, struct bio *,
>  					gfp_t);
> -extern void blk_rq_set_block_pc(struct request *);
> +int blk_rq_set_block_pc(struct request *rq, unsigned short cmd_len,
> +		u8 *sense, gfp_t gfp);
>  extern void blk_requeue_request(struct request_queue *, struct request *);
>  extern void blk_add_request_payload(struct request *rq, struct page *page,
>  		unsigned int len);
<>
> diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
> index 9fc1aec..de6d7d0 100644
> --- a/include/scsi/scsi_cmnd.h
> +++ b/include/scsi/scsi_cmnd.h
> @@ -92,7 +92,7 @@ struct scsi_cmnd {
>  
>  	/* These elements define the operation we are about to perform */
>  	unsigned char *cmnd;
> -
> +	unsigned char __cmnd[32];
>  
>  	/* These elements define the operation we ultimately want to perform */
>  	struct scsi_data_buffer sdb;
<>

Thanks
Boaz


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 7/7] block: allocate block_pc data separate from struct request
  2015-05-06 11:46   ` Boaz Harrosh
@ 2015-05-11  8:00     ` Christoph Hellwig
  0 siblings, 0 replies; 12+ messages in thread
From: Christoph Hellwig @ 2015-05-11  8:00 UTC (permalink / raw)
  To: Boaz Harrosh; +Cc: axboe, linux-nvme, linux-scsi

On Wed, May 06, 2015 at 02:46:18PM +0300, Boaz Harrosh wrote:
> > -	memset(rq->__cmd, 0, sizeof(rq->__cmd));
> > +
> > +	rq->block_pc = kzalloc(sizeof(*rq->block_pc) + cmd_len, gfp);
> 
> I wish you would not embed a dynamic allocation here for any
> driver regardless. This extra allocation does hurt a lot. See how in
> SCSI fs commands you embedded it in scsi_cmnd so it catches as well
> for multi-Q pre-allocation.
> 
> I think you need to just make it the same as *sense pass it from outside
> and the allocation is the caller responsibility. The caller must have
> an end-request call back set. (Or is a sync call)
> 
> Usually all users already have a structure to put this in. The only bit
> more work is to take care of the free. If they are already passing sense
> then they already need to free at end of request. Those that are synchronous
> can have it on the stack. Only very few places need a bit of extra work.

Actually most don't have a structure ready, that's why I ressorted to this
version.  But once this is in you can easily add low-level version that
allows passing a preallocate cdb buffer for the OSD case.

> > +struct block_pc_request {
> > +	unsigned short cmd_len;
> > +	unsigned int sense_len;
> > +	void *sense;
> 
> Better move cmd_len here the short will combine well
> with the char array.

Thanks.

> > +	union {
> > +		struct block_pc_request *block_pc;
> > +		void *drv_private;
> > +	};
> > +
> 
> Exactly. drv_private is allocated by caller we can do the same for
> block_pc. Also If (theoretically) a driver needs both it is just the
> same pointer right? (container_of)

I probably need to rename the field.  It's only driver private when
the low-level driver itself submits the request, e.g. internal commands
in the LLDD.  But in that particular case what you suggest is fine.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2015-05-11  8:00 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-04-17 20:37 RFC: struct request cleanups Christoph Hellwig
2015-04-17 20:37 ` [PATCH 1/7] block: rename REQ_TYPE_SPECIAL to REQ_TYPE_DRV_PRIV Christoph Hellwig
2015-04-17 20:37 ` [PATCH 2/7] block: move REQ_TYPE_ATA_TASKFILE and REQ_TYPE_ATA_PC to ide.h Christoph Hellwig
2015-04-17 20:37 ` [PATCH 3/7] block: move REQ_TYPE_SENSE to the ide driver Christoph Hellwig
2015-04-17 20:37 ` [PATCH 4/7] block: remove REQ_TYPE_PM_SHUTDOWN Christoph Hellwig
2015-04-17 20:37 ` [PATCH 5/7] block: move PM request support to IDE Christoph Hellwig
2015-04-17 20:37 ` [PATCH 6/7] nbd: stop using req->cmd Christoph Hellwig
2015-04-17 20:37 ` [PATCH 7/7] block: allocate block_pc data separate from struct request Christoph Hellwig
2015-05-06 11:46   ` Boaz Harrosh
2015-05-11  8:00     ` Christoph Hellwig
2015-04-21 19:51 ` RFC: struct request cleanups Matthew Wilcox
2015-05-05 19:41 ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox