All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 3/4] scsi: reintroduce scsi_driver.init_command
  2014-03-27 16:14 misc scsi midlayer updates Christoph Hellwig
@ 2014-03-27 16:14 ` Christoph Hellwig
  2014-03-31  5:45   ` Nicholas A. Bellinger
  2014-03-31  6:56   ` Mike Christie
  0 siblings, 2 replies; 9+ messages in thread
From: Christoph Hellwig @ 2014-03-27 16:14 UTC (permalink / raw)
  To: James Bottomley; +Cc: linux-scsi

Move control of the prep_fn back from the ULDs into the scsi layer.  Besides
cleaning up the code and removing the only use of the unprep_fn
requeuest_queue method this also prepares for usinng blk-mq, which doesn't
have equivalent functionality to the prep_fn method and requires the driver
to provide just a single I/O submission method.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/scsi_lib.c    |   66 ++++++++++++++++++++++++++------------------
 drivers/scsi/sd.c          |   46 +++++++++++-------------------
 drivers/scsi/sr.c          |   19 ++++---------
 include/scsi/scsi_driver.h |    8 ++----
 4 files changed, 63 insertions(+), 76 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index a73751b..48c5c77 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -456,6 +456,16 @@ void scsi_requeue_run_queue(struct work_struct *work)
 	scsi_run_queue(q);
 }
 
+static void scsi_uninit_command(struct scsi_cmnd *cmd)
+{
+	if (cmd->request->cmd_type == REQ_TYPE_FS) {
+		struct scsi_driver *drv = scsi_cmd_to_driver(cmd);
+
+		if (drv->uninit_command)
+			drv->uninit_command(cmd);
+	}
+}
+
 /*
  * Function:	scsi_requeue_command()
  *
@@ -480,6 +490,8 @@ static void scsi_requeue_command(struct request_queue *q, struct scsi_cmnd *cmd)
 	struct request *req = cmd->request;
 	unsigned long flags;
 
+	scsi_uninit_command(cmd);
+
 	spin_lock_irqsave(q->queue_lock, flags);
 	blk_unprep_request(req);
 	req->special = NULL;
@@ -1071,15 +1083,7 @@ static struct scsi_cmnd *scsi_get_cmd_from_req(struct scsi_device *sdev,
 
 int scsi_setup_blk_pc_cmnd(struct scsi_device *sdev, struct request *req)
 {
-	struct scsi_cmnd *cmd;
-	int ret = scsi_prep_state_check(sdev, req);
-
-	if (ret != BLKPREP_OK)
-		return ret;
-
-	cmd = scsi_get_cmd_from_req(sdev, req);
-	if (unlikely(!cmd))
-		return BLKPREP_DEFER;
+	struct scsi_cmnd *cmd = req->special;
 
 	/*
 	 * BLOCK_PC requests may transfer data, in which case they must
@@ -1123,15 +1127,11 @@ EXPORT_SYMBOL(scsi_setup_blk_pc_cmnd);
  */
 int scsi_setup_fs_cmnd(struct scsi_device *sdev, struct request *req)
 {
-	struct scsi_cmnd *cmd;
-	int ret = scsi_prep_state_check(sdev, req);
-
-	if (ret != BLKPREP_OK)
-		return ret;
+	struct scsi_cmnd *cmd = req->special;
 
 	if (unlikely(sdev->scsi_dh_data && sdev->scsi_dh_data->scsi_dh
 			 && sdev->scsi_dh_data->scsi_dh->prep_fn)) {
-		ret = sdev->scsi_dh_data->scsi_dh->prep_fn(sdev, req);
+		int ret = sdev->scsi_dh_data->scsi_dh->prep_fn(sdev, req);
 		if (ret != BLKPREP_OK)
 			return ret;
 	}
@@ -1141,16 +1141,13 @@ int scsi_setup_fs_cmnd(struct scsi_device *sdev, struct request *req)
 	 */
 	BUG_ON(!req->nr_phys_segments);
 
-	cmd = scsi_get_cmd_from_req(sdev, req);
-	if (unlikely(!cmd))
-		return BLKPREP_DEFER;
-
 	memset(cmd->cmnd, 0, BLK_MAX_CDB);
 	return scsi_init_io(cmd, GFP_ATOMIC);
 }
 EXPORT_SYMBOL(scsi_setup_fs_cmnd);
 
-int scsi_prep_state_check(struct scsi_device *sdev, struct request *req)
+static int
+scsi_prep_state_check(struct scsi_device *sdev, struct request *req)
 {
 	int ret = BLKPREP_OK;
 
@@ -1202,9 +1199,9 @@ int scsi_prep_state_check(struct scsi_device *sdev, struct request *req)
 	}
 	return ret;
 }
-EXPORT_SYMBOL(scsi_prep_state_check);
 
-int scsi_prep_return(struct request_queue *q, struct request *req, int ret)
+static int
+scsi_prep_return(struct request_queue *q, struct request *req, int ret)
 {
 	struct scsi_device *sdev = q->queuedata;
 
@@ -1235,18 +1232,33 @@ int scsi_prep_return(struct request_queue *q, struct request *req, int ret)
 
 	return ret;
 }
-EXPORT_SYMBOL(scsi_prep_return);
 
-int scsi_prep_fn(struct request_queue *q, struct request *req)
+static int scsi_prep_fn(struct request_queue *q, struct request *req)
 {
 	struct scsi_device *sdev = q->queuedata;
-	int ret = BLKPREP_KILL;
+	struct scsi_cmnd *cmd;
+	int ret;
 
-	if (req->cmd_type == REQ_TYPE_BLOCK_PC)
+	ret = scsi_prep_state_check(sdev, req);
+	if (ret != BLKPREP_OK)
+		goto out;
+
+	cmd = scsi_get_cmd_from_req(sdev, req);
+	if (unlikely(!cmd)) {
+		ret = BLKPREP_DEFER;
+		goto out;
+	}
+
+	if (req->cmd_type == REQ_TYPE_FS)
+		ret = scsi_cmd_to_driver(cmd)->init_command(cmd);
+	else if (req->cmd_type == REQ_TYPE_BLOCK_PC)
 		ret = scsi_setup_blk_pc_cmnd(sdev, req);
+	else
+		ret = BLKPREP_KILL;
+
+out:
 	return scsi_prep_return(q, req, ret);
 }
-EXPORT_SYMBOL(scsi_prep_fn);
 
 /*
  * scsi_dev_queue_ready: if we can send requests to sdev, return 1 else
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 89e6c04..d95c4fd 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -109,6 +109,8 @@ static int sd_suspend_system(struct device *);
 static int sd_suspend_runtime(struct device *);
 static int sd_resume(struct device *);
 static void sd_rescan(struct device *);
+static int sd_init_command(struct scsi_cmnd *SCpnt);
+static void sd_uninit_command(struct scsi_cmnd *SCpnt);
 static int sd_done(struct scsi_cmnd *);
 static int sd_eh_action(struct scsi_cmnd *, int);
 static void sd_read_capacity(struct scsi_disk *sdkp, unsigned char *buffer);
@@ -503,6 +505,8 @@ static struct scsi_driver sd_template = {
 		.pm		= &sd_pm_ops,
 	},
 	.rescan			= sd_rescan,
+	.init_command		= sd_init_command,
+	.uninit_command		= sd_uninit_command,
 	.done			= sd_done,
 	.eh_action		= sd_eh_action,
 };
@@ -838,9 +842,9 @@ static int scsi_setup_flush_cmnd(struct scsi_device *sdp, struct request *rq)
 	return scsi_setup_blk_pc_cmnd(sdp, rq);
 }
 
-static void sd_unprep_fn(struct request_queue *q, struct request *rq)
+static void sd_uninit_command(struct scsi_cmnd *SCpnt)
 {
-	struct scsi_cmnd *SCpnt = rq->special;
+	struct request *rq = SCpnt->request;
 
 	if (rq->cmd_flags & REQ_DISCARD) {
 		free_page((unsigned long)rq->buffer);
@@ -853,18 +857,10 @@ static void sd_unprep_fn(struct request_queue *q, struct request *rq)
 	}
 }
 
-/**
- *	sd_prep_fn - build a scsi (read or write) command from
- *	information in the request structure.
- *	@SCpnt: pointer to mid-level's per scsi command structure that
- *	contains request and into which the scsi command is written
- *
- *	Returns 1 if successful and 0 if error (or cannot be done now).
- **/
-static int sd_prep_fn(struct request_queue *q, struct request *rq)
+static int sd_init_command(struct scsi_cmnd *SCpnt)
 {
-	struct scsi_cmnd *SCpnt;
-	struct scsi_device *sdp = q->queuedata;
+	struct request *rq = SCpnt->request;
+	struct scsi_device *sdp = SCpnt->device;
 	struct gendisk *disk = rq->rq_disk;
 	struct scsi_disk *sdkp;
 	sector_t block = blk_rq_pos(rq);
@@ -886,12 +882,6 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
 	} else if (rq->cmd_flags & REQ_FLUSH) {
 		ret = scsi_setup_flush_cmnd(sdp, rq);
 		goto out;
-	} else if (rq->cmd_type == REQ_TYPE_BLOCK_PC) {
-		ret = scsi_setup_blk_pc_cmnd(sdp, rq);
-		goto out;
-	} else if (rq->cmd_type != REQ_TYPE_FS) {
-		ret = BLKPREP_KILL;
-		goto out;
 	}
 	ret = scsi_setup_fs_cmnd(sdp, rq);
 	if (ret != BLKPREP_OK)
@@ -903,11 +893,10 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
 	 * is used for a killable error condition */
 	ret = BLKPREP_KILL;
 
-	SCSI_LOG_HLQUEUE(1, scmd_printk(KERN_INFO, SCpnt,
-					"sd_prep_fn: block=%llu, "
-					"count=%d\n",
-					(unsigned long long)block,
-					this_count));
+	SCSI_LOG_HLQUEUE(1,
+		scmd_printk(KERN_INFO, SCpnt,
+			"%s: block=%llu, count=%d\n",
+			__func__, (unsigned long long)block, this_count));
 
 	if (!sdp || !scsi_device_online(sdp) ||
 	    block + blk_rq_sectors(rq) > get_capacity(disk)) {
@@ -1127,7 +1116,7 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
 	 */
 	ret = BLKPREP_OK;
  out:
-	return scsi_prep_return(q, rq, ret);
+	return ret;
 }
 
 /**
@@ -1663,6 +1652,8 @@ static int sd_done(struct scsi_cmnd *SCpnt)
 	unsigned char op = SCpnt->cmnd[0];
 	unsigned char unmap = SCpnt->cmnd[1] & 8;
 
+	sd_uninit_command(SCpnt);
+
 	if (req->cmd_flags & REQ_DISCARD || req->cmd_flags & REQ_WRITE_SAME) {
 		if (!result) {
 			good_bytes = blk_rq_bytes(req);
@@ -2878,9 +2869,6 @@ static void sd_probe_async(void *data, async_cookie_t cookie)
 
 	sd_revalidate_disk(gd);
 
-	blk_queue_prep_rq(sdp->request_queue, sd_prep_fn);
-	blk_queue_unprep_rq(sdp->request_queue, sd_unprep_fn);
-
 	gd->driverfs_dev = &sdp->sdev_gendev;
 	gd->flags = GENHD_FL_EXT_DEVT;
 	if (sdp->removable) {
@@ -3027,8 +3015,6 @@ static int sd_remove(struct device *dev)
 	scsi_autopm_get_device(sdkp->device);
 
 	async_synchronize_full_domain(&scsi_sd_probe_domain);
-	blk_queue_prep_rq(sdkp->device->request_queue, scsi_prep_fn);
-	blk_queue_unprep_rq(sdkp->device->request_queue, NULL);
 	device_del(&sdkp->dev);
 	del_gendisk(sdkp->disk);
 	sd_shutdown(dev);
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 40d8592..93cbd36 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -79,6 +79,7 @@ MODULE_ALIAS_SCSI_DEVICE(TYPE_WORM);
 static DEFINE_MUTEX(sr_mutex);
 static int sr_probe(struct device *);
 static int sr_remove(struct device *);
+static int sr_init_command(struct scsi_cmnd *SCpnt);
 static int sr_done(struct scsi_cmnd *);
 static int sr_runtime_suspend(struct device *dev);
 
@@ -94,6 +95,7 @@ static struct scsi_driver sr_template = {
 		.remove		= sr_remove,
 		.pm		= &sr_pm_ops,
 	},
+	.init_command		= sr_init_command,
 	.done			= sr_done,
 };
 
@@ -378,21 +380,14 @@ static int sr_done(struct scsi_cmnd *SCpnt)
 	return good_bytes;
 }
 
-static int sr_prep_fn(struct request_queue *q, struct request *rq)
+static int sr_init_command(struct scsi_cmnd *SCpnt)
 {
 	int block = 0, this_count, s_size;
 	struct scsi_cd *cd;
-	struct scsi_cmnd *SCpnt;
-	struct scsi_device *sdp = q->queuedata;
+	struct request *rq = SCpnt->request;
+	struct scsi_device *sdp = SCpnt->device;
 	int ret;
 
-	if (rq->cmd_type == REQ_TYPE_BLOCK_PC) {
-		ret = scsi_setup_blk_pc_cmnd(sdp, rq);
-		goto out;
-	} else if (rq->cmd_type != REQ_TYPE_FS) {
-		ret = BLKPREP_KILL;
-		goto out;
-	}
 	ret = scsi_setup_fs_cmnd(sdp, rq);
 	if (ret != BLKPREP_OK)
 		goto out;
@@ -517,7 +512,7 @@ static int sr_prep_fn(struct request_queue *q, struct request *rq)
 	 */
 	ret = BLKPREP_OK;
  out:
-	return scsi_prep_return(q, rq, ret);
+	return ret;
 }
 
 static int sr_block_open(struct block_device *bdev, fmode_t mode)
@@ -718,7 +713,6 @@ static int sr_probe(struct device *dev)
 
 	/* FIXME: need to handle a get_capabilities failure properly ?? */
 	get_capabilities(cd);
-	blk_queue_prep_rq(sdev->request_queue, sr_prep_fn);
 	sr_vendor_init(cd);
 
 	disk->driverfs_dev = &sdev->sdev_gendev;
@@ -993,7 +987,6 @@ static int sr_remove(struct device *dev)
 
 	scsi_autopm_get_device(cd->device);
 
-	blk_queue_prep_rq(cd->device->request_queue, scsi_prep_fn);
 	del_gendisk(cd->disk);
 
 	mutex_lock(&sr_ref_mutex);
diff --git a/include/scsi/scsi_driver.h b/include/scsi/scsi_driver.h
index 20fdfc2..b507729 100644
--- a/include/scsi/scsi_driver.h
+++ b/include/scsi/scsi_driver.h
@@ -6,15 +6,14 @@
 struct module;
 struct scsi_cmnd;
 struct scsi_device;
-struct request;
-struct request_queue;
-
 
 struct scsi_driver {
 	struct module		*owner;
 	struct device_driver	gendrv;
 
 	void (*rescan)(struct device *);
+	int (*init_command)(struct scsi_cmnd *);
+	void (*uninit_command)(struct scsi_cmnd *);
 	int (*done)(struct scsi_cmnd *);
 	int (*eh_action)(struct scsi_cmnd *, int);
 };
@@ -31,8 +30,5 @@ extern int scsi_register_interface(struct class_interface *);
 
 int scsi_setup_blk_pc_cmnd(struct scsi_device *sdev, struct request *req);
 int scsi_setup_fs_cmnd(struct scsi_device *sdev, struct request *req);
-int scsi_prep_state_check(struct scsi_device *sdev, struct request *req);
-int scsi_prep_return(struct request_queue *q, struct request *req, int ret);
-int scsi_prep_fn(struct request_queue *, struct request *);
 
 #endif /* _SCSI_SCSI_DRIVER_H */
-- 
1.7.10.4


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

* Re: [PATCH 3/4] scsi: reintroduce scsi_driver.init_command
  2014-03-27 16:14 ` [PATCH 3/4] scsi: reintroduce scsi_driver.init_command Christoph Hellwig
@ 2014-03-31  5:45   ` Nicholas A. Bellinger
  2014-03-31  6:08     ` Christoph Hellwig
  2014-03-31  6:56   ` Mike Christie
  1 sibling, 1 reply; 9+ messages in thread
From: Nicholas A. Bellinger @ 2014-03-31  5:45 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: James Bottomley, linux-scsi

On Thu, 2014-03-27 at 17:14 +0100, Christoph Hellwig wrote:
> Move control of the prep_fn back from the ULDs into the scsi layer.  Besides
> cleaning up the code and removing the only use of the unprep_fn
> requeuest_queue method this also prepares for usinng blk-mq, which doesn't
> have equivalent functionality to the prep_fn method and requires the driver
> to provide just a single I/O submission method.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/scsi/scsi_lib.c    |   66 ++++++++++++++++++++++++++------------------
>  drivers/scsi/sd.c          |   46 +++++++++++-------------------
>  drivers/scsi/sr.c          |   19 ++++---------
>  include/scsi/scsi_driver.h |    8 ++----
>  4 files changed, 63 insertions(+), 76 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
> index a73751b..48c5c77 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c

<SNIP>

> @@ -1071,15 +1083,7 @@ static struct scsi_cmnd *scsi_get_cmd_from_req(struct scsi_device *sdev,
>  
>  int scsi_setup_blk_pc_cmnd(struct scsi_device *sdev, struct request *req)
>  {
> -	struct scsi_cmnd *cmd;
> -	int ret = scsi_prep_state_check(sdev, req);
> -
> -	if (ret != BLKPREP_OK)
> -		return ret;
> -
> -	cmd = scsi_get_cmd_from_req(sdev, req);
> -	if (unlikely(!cmd))
> -		return BLKPREP_DEFER;
> +	struct scsi_cmnd *cmd = req->special;
>  

Mmm, I thought that req->special was only holding a pointer to a
pre-allocated scsi_cmnd during mq operation, no..?

>  	/*
>  	 * BLOCK_PC requests may transfer data, in which case they must
> @@ -1123,15 +1127,11 @@ EXPORT_SYMBOL(scsi_setup_blk_pc_cmnd);
>   */
>  int scsi_setup_fs_cmnd(struct scsi_device *sdev, struct request *req)
>  {
> -	struct scsi_cmnd *cmd;
> -	int ret = scsi_prep_state_check(sdev, req);
> -
> -	if (ret != BLKPREP_OK)
> -		return ret;
> +	struct scsi_cmnd *cmd = req->special;
>  

Ditto here for REQ_TYPE_FS_CMND

>  	if (unlikely(sdev->scsi_dh_data && sdev->scsi_dh_data->scsi_dh
>  			 && sdev->scsi_dh_data->scsi_dh->prep_fn)) {
> -		ret = sdev->scsi_dh_data->scsi_dh->prep_fn(sdev, req);
> +		int ret = sdev->scsi_dh_data->scsi_dh->prep_fn(sdev, req);
>  		if (ret != BLKPREP_OK)
>  			return ret;
>  	}
> @@ -1141,16 +1141,13 @@ int scsi_setup_fs_cmnd(struct scsi_device *sdev, struct request *req)
>  	 */
>  	BUG_ON(!req->nr_phys_segments);
>  
> -	cmd = scsi_get_cmd_from_req(sdev, req);
> -	if (unlikely(!cmd))
> -		return BLKPREP_DEFER;
> -
>  	memset(cmd->cmnd, 0, BLK_MAX_CDB);
>  	return scsi_init_io(cmd, GFP_ATOMIC);
>  }
>  EXPORT_SYMBOL(scsi_setup_fs_cmnd);
>  
> -int scsi_prep_state_check(struct scsi_device *sdev, struct request *req)
> +static int
> +scsi_prep_state_check(struct scsi_device *sdev, struct request *req)
>  {
>  	int ret = BLKPREP_OK;
>  
> @@ -1202,9 +1199,9 @@ int scsi_prep_state_check(struct scsi_device *sdev, struct request *req)
>  	}
>  	return ret;
>  }
> -EXPORT_SYMBOL(scsi_prep_state_check);
>  
> -int scsi_prep_return(struct request_queue *q, struct request *req, int ret)
> +static int
> +scsi_prep_return(struct request_queue *q, struct request *req, int ret)
>  {
>  	struct scsi_device *sdev = q->queuedata;
>  
> @@ -1235,18 +1232,33 @@ int scsi_prep_return(struct request_queue *q, struct request *req, int ret)
>  
>  	return ret;
>  }
> -EXPORT_SYMBOL(scsi_prep_return);
>  
> -int scsi_prep_fn(struct request_queue *q, struct request *req)
> +static int scsi_prep_fn(struct request_queue *q, struct request *req)
>  {
>  	struct scsi_device *sdev = q->queuedata;
> -	int ret = BLKPREP_KILL;
> +	struct scsi_cmnd *cmd;
> +	int ret;
>  
> -	if (req->cmd_type == REQ_TYPE_BLOCK_PC)
> +	ret = scsi_prep_state_check(sdev, req);
> +	if (ret != BLKPREP_OK)
> +		goto out;
> +
> +	cmd = scsi_get_cmd_from_req(sdev, req);
> +	if (unlikely(!cmd)) {
> +		ret = BLKPREP_DEFER;
> +		goto out;
> +	}
> +

>From here the req->special pointer may or may not be set depending on mq
operation, no..?

> +	if (req->cmd_type == REQ_TYPE_FS)
> +		ret = scsi_cmd_to_driver(cmd)->init_command(cmd);
> +	else if (req->cmd_type == REQ_TYPE_BLOCK_PC)
>  		ret = scsi_setup_blk_pc_cmnd(sdev, req);
> +	else
> +		ret = BLKPREP_KILL;
> +
> +out:
>  	return scsi_prep_return(q, req, ret);
>  }
> -EXPORT_SYMBOL(scsi_prep_fn);
>  
>  /*
>   * scsi_dev_queue_ready: if we can send requests to sdev, return 1 else
> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 89e6c04..d95c4fd 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c

<SNIP>

> @@ -886,12 +882,6 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
>  	} else if (rq->cmd_flags & REQ_FLUSH) {
>  		ret = scsi_setup_flush_cmnd(sdp, rq);
>  		goto out;
> -	} else if (rq->cmd_type == REQ_TYPE_BLOCK_PC) {
> -		ret = scsi_setup_blk_pc_cmnd(sdp, rq);
> -		goto out;
> -	} else if (rq->cmd_type != REQ_TYPE_FS) {
> -		ret = BLKPREP_KILL;
> -		goto out;
>  	}
>  	ret = scsi_setup_fs_cmnd(sdp, rq);
>  	if (ret != BLKPREP_OK)

And this currently assumes req->special is always set when calling
scsi_setup_fs_cmnd()

> @@ -903,11 +893,10 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
>  	 * is used for a killable error condition */
>  	ret = BLKPREP_KILL;
>  
> -	SCSI_LOG_HLQUEUE(1, scmd_printk(KERN_INFO, SCpnt,
> -					"sd_prep_fn: block=%llu, "
> -					"count=%d\n",
> -					(unsigned long long)block,
> -					this_count));
> +	SCSI_LOG_HLQUEUE(1,
> +		scmd_printk(KERN_INFO, SCpnt,
> +			"%s: block=%llu, count=%d\n",
> +			__func__, (unsigned long long)block, this_count));
>  
>  	if (!sdp || !scsi_device_online(sdp) ||
>  	    block + blk_rq_sectors(rq) > get_capacity(disk)) {
> @@ -1127,7 +1116,7 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
>  	 */
>  	ret = BLKPREP_OK;
>   out:
> -	return scsi_prep_return(q, rq, ret);
> +	return ret;
>  }
>  
>  /**
> @@ -1663,6 +1652,8 @@ static int sd_done(struct scsi_cmnd *SCpnt)
>  	unsigned char op = SCpnt->cmnd[0];
>  	unsigned char unmap = SCpnt->cmnd[1] & 8;
>  
> +	sd_uninit_command(SCpnt);
> +
>  	if (req->cmd_flags & REQ_DISCARD || req->cmd_flags & REQ_WRITE_SAME) {
>  		if (!result) {
>  			good_bytes = blk_rq_bytes(req);
> @@ -2878,9 +2869,6 @@ static void sd_probe_async(void *data, async_cookie_t cookie)
>  
>  	sd_revalidate_disk(gd);
>  
> -	blk_queue_prep_rq(sdp->request_queue, sd_prep_fn);
> -	blk_queue_unprep_rq(sdp->request_queue, sd_unprep_fn);
> -
>  	gd->driverfs_dev = &sdp->sdev_gendev;
>  	gd->flags = GENHD_FL_EXT_DEVT;
>  	if (sdp->removable) {
> @@ -3027,8 +3015,6 @@ static int sd_remove(struct device *dev)
>  	scsi_autopm_get_device(sdkp->device);
>  
>  	async_synchronize_full_domain(&scsi_sd_probe_domain);
> -	blk_queue_prep_rq(sdkp->device->request_queue, scsi_prep_fn);
> -	blk_queue_unprep_rq(sdkp->device->request_queue, NULL);
>  	device_del(&sdkp->dev);
>  	del_gendisk(sdkp->disk);
>  	sd_shutdown(dev);
> diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
> index 40d8592..93cbd36 100644
> --- a/drivers/scsi/sr.c
> +++ b/drivers/scsi/sr.c

<SNIP>

> @@ -378,21 +380,14 @@ static int sr_done(struct scsi_cmnd *SCpnt)
>  	return good_bytes;
>  }
>  
> -static int sr_prep_fn(struct request_queue *q, struct request *rq)
> +static int sr_init_command(struct scsi_cmnd *SCpnt)
>  {
>  	int block = 0, this_count, s_size;
>  	struct scsi_cd *cd;
> -	struct scsi_cmnd *SCpnt;
> -	struct scsi_device *sdp = q->queuedata;
> +	struct request *rq = SCpnt->request;
> +	struct scsi_device *sdp = SCpnt->device;
>  	int ret;
>  
> -	if (rq->cmd_type == REQ_TYPE_BLOCK_PC) {
> -		ret = scsi_setup_blk_pc_cmnd(sdp, rq);
> -		goto out;
> -	} else if (rq->cmd_type != REQ_TYPE_FS) {
> -		ret = BLKPREP_KILL;
> -		goto out;
> -	}
>  	ret = scsi_setup_fs_cmnd(sdp, rq);
>  	if (ret != BLKPREP_OK)
>  		goto out;

Ditto to the scsi_setup_fs_cmnd() call here as well.

--nab


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

* Re: [PATCH 3/4] scsi: reintroduce scsi_driver.init_command
  2014-03-31  5:45   ` Nicholas A. Bellinger
@ 2014-03-31  6:08     ` Christoph Hellwig
  2014-03-31  6:20       ` Nicholas A. Bellinger
  0 siblings, 1 reply; 9+ messages in thread
From: Christoph Hellwig @ 2014-03-31  6:08 UTC (permalink / raw)
  To: Nicholas A. Bellinger; +Cc: Christoph Hellwig, James Bottomley, linux-scsi

On Sun, Mar 30, 2014 at 10:45:04PM -0700, Nicholas A. Bellinger wrote:
> >  int scsi_setup_blk_pc_cmnd(struct scsi_device *sdev, struct request *req)
> >  {
> > -	struct scsi_cmnd *cmd;
> > -	int ret = scsi_prep_state_check(sdev, req);
> > -
> > -	if (ret != BLKPREP_OK)
> > -		return ret;
> > -
> > -	cmd = scsi_get_cmd_from_req(sdev, req);
> > -	if (unlikely(!cmd))
> > -		return BLKPREP_DEFER;
> > +	struct scsi_cmnd *cmd = req->special;
> >  
> 
> Mmm, I thought that req->special was only holding a pointer to a
> pre-allocated scsi_cmnd during mq operation, no..?

For the mq case the block layer sets up req->special.  For the old case
scsi_get_cmd_from_req sets up req->special the first it is called, and
leaves it in there until the command is done.

> > +	ret = scsi_prep_state_check(sdev, req);
> > +	if (ret != BLKPREP_OK)
> > +		goto out;
> > +
> > +	cmd = scsi_get_cmd_from_req(sdev, req);
> > +	if (unlikely(!cmd)) {
> > +		ret = BLKPREP_DEFER;
> > +		goto out;
> > +	}
> > +


>From here the req->special pointer may or may not be set depending on mq
operation, no..?

a) there is no blk-mq support yet in James tree (+ these patches)
b) even in my scsi-mq tree this code path won't be called for the mq case
c) after scsi_get_cmd_from_req returns req->special will always be set up
   to point to the scsi_cmnd:

static struct scsi_cmnd *scsi_get_cmd_from_req(struct scsi_device *sdev,
		struct request *req)
{
	struct scsi_cmnd *cmd;

	if (!req->special) {
		...

		req->special = cmd;
	} else {
		cmd = req->special;
	}

        /* pull a tag out of the request if we have one */
	cmd->tag = req->tag;
	cmd->request = req;

	cmd->cmnd = req->cmd;   
	cmd->prot_op = SCSI_PROT_NORMAL;

	return cmd;
}

> >  	}
> >  	ret = scsi_setup_fs_cmnd(sdp, rq);
> >  	if (ret != BLKPREP_OK)
> 
> And this currently assumes req->special is always set when calling
> scsi_setup_fs_cmnd()

That is the case both before and after this patch.


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

* Re: [PATCH 3/4] scsi: reintroduce scsi_driver.init_command
  2014-03-31  6:08     ` Christoph Hellwig
@ 2014-03-31  6:20       ` Nicholas A. Bellinger
  0 siblings, 0 replies; 9+ messages in thread
From: Nicholas A. Bellinger @ 2014-03-31  6:20 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: James Bottomley, linux-scsi

On Mon, 2014-03-31 at 08:08 +0200, Christoph Hellwig wrote:
> On Sun, Mar 30, 2014 at 10:45:04PM -0700, Nicholas A. Bellinger wrote:
> > >  int scsi_setup_blk_pc_cmnd(struct scsi_device *sdev, struct request *req)
> > >  {
> > > -	struct scsi_cmnd *cmd;
> > > -	int ret = scsi_prep_state_check(sdev, req);
> > > -
> > > -	if (ret != BLKPREP_OK)
> > > -		return ret;
> > > -
> > > -	cmd = scsi_get_cmd_from_req(sdev, req);
> > > -	if (unlikely(!cmd))
> > > -		return BLKPREP_DEFER;
> > > +	struct scsi_cmnd *cmd = req->special;
> > >  
> > 
> > Mmm, I thought that req->special was only holding a pointer to a
> > pre-allocated scsi_cmnd during mq operation, no..?
> 
> For the mq case the block layer sets up req->special.  For the old case
> scsi_get_cmd_from_req sets up req->special the first it is called, and
> leaves it in there until the command is done.

Er, yes of course.

Reviewed-by: Nicholas Bellinger <nab@linux-iscsi.org>


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

* Re: [PATCH 3/4] scsi: reintroduce scsi_driver.init_command
  2014-03-27 16:14 ` [PATCH 3/4] scsi: reintroduce scsi_driver.init_command Christoph Hellwig
  2014-03-31  5:45   ` Nicholas A. Bellinger
@ 2014-03-31  6:56   ` Mike Christie
  2014-03-31  8:09     ` Christoph Hellwig
  1 sibling, 1 reply; 9+ messages in thread
From: Mike Christie @ 2014-03-31  6:56 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: James Bottomley, linux-scsi

On 03/27/2014 11:14 AM, Christoph Hellwig wrote:
> @@ -1663,6 +1652,8 @@ static int sd_done(struct scsi_cmnd *SCpnt)
>  	unsigned char op = SCpnt->cmnd[0];
>  	unsigned char unmap = SCpnt->cmnd[1] & 8;
>  
> +	sd_uninit_command(SCpnt);
> +

The above call would free the cmnd->cmnd and set it to null. If then
scsi_io_completion was going to do some error processing it looks like
it could try to access the scsi_cmnd->cmnd field.

With the current code that would not be a problem because the blk unprep
callback is not called until the block layer does its request cleanup in
blk_finish_request which as you know is after
scsi_io_completion/scsi_end_request is done with the cmnd.

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

* Re: [PATCH 3/4] scsi: reintroduce scsi_driver.init_command
  2014-03-31  6:56   ` Mike Christie
@ 2014-03-31  8:09     ` Christoph Hellwig
  0 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2014-03-31  8:09 UTC (permalink / raw)
  To: Mike Christie; +Cc: Christoph Hellwig, James Bottomley, linux-scsi

On Mon, Mar 31, 2014 at 01:56:13AM -0500, Mike Christie wrote:
> The above call would free the cmnd->cmnd and set it to null. If then
> scsi_io_completion was going to do some error processing it looks like
> it could try to access the scsi_cmnd->cmnd field.
> 
> With the current code that would not be a problem because the blk unprep
> callback is not called until the block layer does its request cleanup in
> blk_finish_request which as you know is after
> scsi_io_completion/scsi_end_request is done with the cmnd.

Right, we should probabl call ->uninit_command at that place as well
instead of calling it internall in ->done.

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

* Re: [PATCH 3/4] scsi: reintroduce scsi_driver.init_command
@ 2014-03-31 10:45 Christoph Hellwig
  0 siblings, 0 replies; 9+ messages in thread
From: Christoph Hellwig @ 2014-03-31 10:45 UTC (permalink / raw)
  To: Mike Christie; +Cc: James Bottomley, linux-scsi

> The above call would free the cmnd->cmnd and set it to null. If then
> scsi_io_completion was going to do some error processing it looks like
> it could try to access the scsi_cmnd->cmnd field.
> 
> With the current code that would not be a problem because the blk unprep
> callback is not called until the block layer does its request cleanup in
> blk_finish_request which as you know is after
> scsi_io_completion/scsi_end_request is done with the cmnd.

This incremental patches fixes the issue, and makes sure the uninit calls are
nicely paired like the rest of the I/O completion routines after patch 2:


diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 48c5c77..8e79612 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -490,8 +490,6 @@ static void scsi_requeue_command(struct request_queue *q, struct scsi_cmnd *cmd)
 	struct request *req = cmd->request;
 	unsigned long flags;
 
-	scsi_uninit_command(cmd);
-
 	spin_lock_irqsave(q->queue_lock, flags);
 	blk_unprep_request(req);
 	req->special = NULL;
@@ -941,6 +939,7 @@ requeue:
 		/* Unprep the request and put it back at the head of the queue.
 		 * A new command will be prepared and issued.
 		 */
+		scsi_uninit_command(cmd);
 		scsi_release_buffers(cmd);
 		scsi_requeue_command(q, cmd);
 		break;
@@ -956,6 +955,7 @@ requeue:
 	return;
 
 next_command:
+	scsi_uninit_command(cmd);
 	scsi_release_buffers(cmd);
 	scsi_next_command(cmd);
 }
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index d95c4fd..d99cb3f 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -1652,8 +1652,6 @@ static int sd_done(struct scsi_cmnd *SCpnt)
 	unsigned char op = SCpnt->cmnd[0];
 	unsigned char unmap = SCpnt->cmnd[1] & 8;
 
-	sd_uninit_command(SCpnt);
-
 	if (req->cmd_flags & REQ_DISCARD || req->cmd_flags & REQ_WRITE_SAME) {
 		if (!result) {
 			good_bytes = blk_rq_bytes(req);

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

* [PATCH 3/4] scsi: reintroduce scsi_driver.init_command
  2014-05-01 14:51 scsi midlayer updates for 3.15 (resend) Christoph Hellwig
@ 2014-05-01 14:51 ` Christoph Hellwig
  2014-05-08  7:11   ` Hannes Reinecke
  0 siblings, 1 reply; 9+ messages in thread
From: Christoph Hellwig @ 2014-05-01 14:51 UTC (permalink / raw)
  To: James Bottomley; +Cc: linux-scsi

Instead of letting the ULD play games with the prep_fn move back to
the model of a central prep_fn with a callback to the ULD.  This
already cleans up and shortens the code by itself, and will be required
to properly support blk-mq in the SCSI midlayer.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Nicholas Bellinger <nab@linux-iscsi.org>
Reviewed-by: Mike Christie <michaelc@cs.wisc.edu>
---
 drivers/scsi/scsi_lib.c    |   66 ++++++++++++++++++++++++++------------------
 drivers/scsi/sd.c          |   44 ++++++++++-------------------
 drivers/scsi/sr.c          |   19 ++++---------
 include/scsi/scsi_driver.h |    9 ++----
 4 files changed, 62 insertions(+), 76 deletions(-)

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 045822b..9f841df 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -1073,15 +1073,7 @@ static struct scsi_cmnd *scsi_get_cmd_from_req(struct scsi_device *sdev,
 
 int scsi_setup_blk_pc_cmnd(struct scsi_device *sdev, struct request *req)
 {
-	struct scsi_cmnd *cmd;
-	int ret = scsi_prep_state_check(sdev, req);
-
-	if (ret != BLKPREP_OK)
-		return ret;
-
-	cmd = scsi_get_cmd_from_req(sdev, req);
-	if (unlikely(!cmd))
-		return BLKPREP_DEFER;
+	struct scsi_cmnd *cmd = req->special;
 
 	/*
 	 * BLOCK_PC requests may transfer data, in which case they must
@@ -1125,15 +1117,11 @@ EXPORT_SYMBOL(scsi_setup_blk_pc_cmnd);
  */
 int scsi_setup_fs_cmnd(struct scsi_device *sdev, struct request *req)
 {
-	struct scsi_cmnd *cmd;
-	int ret = scsi_prep_state_check(sdev, req);
-
-	if (ret != BLKPREP_OK)
-		return ret;
+	struct scsi_cmnd *cmd = req->special;
 
 	if (unlikely(sdev->scsi_dh_data && sdev->scsi_dh_data->scsi_dh
 			 && sdev->scsi_dh_data->scsi_dh->prep_fn)) {
-		ret = sdev->scsi_dh_data->scsi_dh->prep_fn(sdev, req);
+		int ret = sdev->scsi_dh_data->scsi_dh->prep_fn(sdev, req);
 		if (ret != BLKPREP_OK)
 			return ret;
 	}
@@ -1143,16 +1131,13 @@ int scsi_setup_fs_cmnd(struct scsi_device *sdev, struct request *req)
 	 */
 	BUG_ON(!req->nr_phys_segments);
 
-	cmd = scsi_get_cmd_from_req(sdev, req);
-	if (unlikely(!cmd))
-		return BLKPREP_DEFER;
-
 	memset(cmd->cmnd, 0, BLK_MAX_CDB);
 	return scsi_init_io(cmd, GFP_ATOMIC);
 }
 EXPORT_SYMBOL(scsi_setup_fs_cmnd);
 
-int scsi_prep_state_check(struct scsi_device *sdev, struct request *req)
+static int
+scsi_prep_state_check(struct scsi_device *sdev, struct request *req)
 {
 	int ret = BLKPREP_OK;
 
@@ -1204,9 +1189,9 @@ int scsi_prep_state_check(struct scsi_device *sdev, struct request *req)
 	}
 	return ret;
 }
-EXPORT_SYMBOL(scsi_prep_state_check);
 
-int scsi_prep_return(struct request_queue *q, struct request *req, int ret)
+static int
+scsi_prep_return(struct request_queue *q, struct request *req, int ret)
 {
 	struct scsi_device *sdev = q->queuedata;
 
@@ -1237,18 +1222,44 @@ int scsi_prep_return(struct request_queue *q, struct request *req, int ret)
 
 	return ret;
 }
-EXPORT_SYMBOL(scsi_prep_return);
 
-int scsi_prep_fn(struct request_queue *q, struct request *req)
+static int scsi_prep_fn(struct request_queue *q, struct request *req)
 {
 	struct scsi_device *sdev = q->queuedata;
-	int ret = BLKPREP_KILL;
+	struct scsi_cmnd *cmd;
+	int ret;
 
-	if (req->cmd_type == REQ_TYPE_BLOCK_PC)
+	ret = scsi_prep_state_check(sdev, req);
+	if (ret != BLKPREP_OK)
+		goto out;
+
+	cmd = scsi_get_cmd_from_req(sdev, req);
+	if (unlikely(!cmd)) {
+		ret = BLKPREP_DEFER;
+		goto out;
+	}
+
+	if (req->cmd_type == REQ_TYPE_FS)
+		ret = scsi_cmd_to_driver(cmd)->init_command(cmd);
+	else if (req->cmd_type == REQ_TYPE_BLOCK_PC)
 		ret = scsi_setup_blk_pc_cmnd(sdev, req);
+	else
+		ret = BLKPREP_KILL;
+
+out:
 	return scsi_prep_return(q, req, ret);
 }
-EXPORT_SYMBOL(scsi_prep_fn);
+
+static void scsi_unprep_fn(struct request_queue *q, struct request *req)
+{
+	if (req->cmd_type == REQ_TYPE_FS) {
+		struct scsi_cmnd *cmd = req->special;
+		struct scsi_driver *drv = scsi_cmd_to_driver(cmd);
+
+		if (drv->uninit_command)
+			drv->uninit_command(cmd);
+	}
+}
 
 /*
  * scsi_dev_queue_ready: if we can send requests to sdev, return 1 else
@@ -1669,6 +1680,7 @@ struct request_queue *scsi_alloc_queue(struct scsi_device *sdev)
 		return NULL;
 
 	blk_queue_prep_rq(q, scsi_prep_fn);
+	blk_queue_unprep_rq(q, scsi_unprep_fn);
 	blk_queue_softirq_done(q, scsi_softirq_done);
 	blk_queue_rq_timed_out(q, scsi_times_out);
 	blk_queue_lld_busy(q, scsi_lld_busy);
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index efcbcd1..1c1253b 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -109,6 +109,8 @@ static int sd_suspend_system(struct device *);
 static int sd_suspend_runtime(struct device *);
 static int sd_resume(struct device *);
 static void sd_rescan(struct device *);
+static int sd_init_command(struct scsi_cmnd *SCpnt);
+static void sd_uninit_command(struct scsi_cmnd *SCpnt);
 static int sd_done(struct scsi_cmnd *);
 static int sd_eh_action(struct scsi_cmnd *, int);
 static void sd_read_capacity(struct scsi_disk *sdkp, unsigned char *buffer);
@@ -503,6 +505,8 @@ static struct scsi_driver sd_template = {
 		.pm		= &sd_pm_ops,
 	},
 	.rescan			= sd_rescan,
+	.init_command		= sd_init_command,
+	.uninit_command		= sd_uninit_command,
 	.done			= sd_done,
 	.eh_action		= sd_eh_action,
 };
@@ -838,9 +842,9 @@ static int scsi_setup_flush_cmnd(struct scsi_device *sdp, struct request *rq)
 	return scsi_setup_blk_pc_cmnd(sdp, rq);
 }
 
-static void sd_unprep_fn(struct request_queue *q, struct request *rq)
+static void sd_uninit_command(struct scsi_cmnd *SCpnt)
 {
-	struct scsi_cmnd *SCpnt = rq->special;
+	struct request *rq = SCpnt->request;
 
 	if (rq->cmd_flags & REQ_DISCARD) {
 		free_page((unsigned long)rq->buffer);
@@ -853,18 +857,10 @@ static void sd_unprep_fn(struct request_queue *q, struct request *rq)
 	}
 }
 
-/**
- *	sd_prep_fn - build a scsi (read or write) command from
- *	information in the request structure.
- *	@SCpnt: pointer to mid-level's per scsi command structure that
- *	contains request and into which the scsi command is written
- *
- *	Returns 1 if successful and 0 if error (or cannot be done now).
- **/
-static int sd_prep_fn(struct request_queue *q, struct request *rq)
+static int sd_init_command(struct scsi_cmnd *SCpnt)
 {
-	struct scsi_cmnd *SCpnt;
-	struct scsi_device *sdp = q->queuedata;
+	struct request *rq = SCpnt->request;
+	struct scsi_device *sdp = SCpnt->device;
 	struct gendisk *disk = rq->rq_disk;
 	struct scsi_disk *sdkp;
 	sector_t block = blk_rq_pos(rq);
@@ -886,12 +882,6 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
 	} else if (rq->cmd_flags & REQ_FLUSH) {
 		ret = scsi_setup_flush_cmnd(sdp, rq);
 		goto out;
-	} else if (rq->cmd_type == REQ_TYPE_BLOCK_PC) {
-		ret = scsi_setup_blk_pc_cmnd(sdp, rq);
-		goto out;
-	} else if (rq->cmd_type != REQ_TYPE_FS) {
-		ret = BLKPREP_KILL;
-		goto out;
 	}
 	ret = scsi_setup_fs_cmnd(sdp, rq);
 	if (ret != BLKPREP_OK)
@@ -903,11 +893,10 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
 	 * is used for a killable error condition */
 	ret = BLKPREP_KILL;
 
-	SCSI_LOG_HLQUEUE(1, scmd_printk(KERN_INFO, SCpnt,
-					"sd_prep_fn: block=%llu, "
-					"count=%d\n",
-					(unsigned long long)block,
-					this_count));
+	SCSI_LOG_HLQUEUE(1,
+		scmd_printk(KERN_INFO, SCpnt,
+			"%s: block=%llu, count=%d\n",
+			__func__, (unsigned long long)block, this_count));
 
 	if (!sdp || !scsi_device_online(sdp) ||
 	    block + blk_rq_sectors(rq) > get_capacity(disk)) {
@@ -1127,7 +1116,7 @@ static int sd_prep_fn(struct request_queue *q, struct request *rq)
 	 */
 	ret = BLKPREP_OK;
  out:
-	return scsi_prep_return(q, rq, ret);
+ 	return ret;
 }
 
 /**
@@ -2878,9 +2867,6 @@ static void sd_probe_async(void *data, async_cookie_t cookie)
 
 	sd_revalidate_disk(gd);
 
-	blk_queue_prep_rq(sdp->request_queue, sd_prep_fn);
-	blk_queue_unprep_rq(sdp->request_queue, sd_unprep_fn);
-
 	gd->driverfs_dev = &sdp->sdev_gendev;
 	gd->flags = GENHD_FL_EXT_DEVT;
 	if (sdp->removable) {
@@ -3028,8 +3014,6 @@ static int sd_remove(struct device *dev)
 
 	async_synchronize_full_domain(&scsi_sd_pm_domain);
 	async_synchronize_full_domain(&scsi_sd_probe_domain);
-	blk_queue_prep_rq(sdkp->device->request_queue, scsi_prep_fn);
-	blk_queue_unprep_rq(sdkp->device->request_queue, NULL);
 	device_del(&sdkp->dev);
 	del_gendisk(sdkp->disk);
 	sd_shutdown(dev);
diff --git a/drivers/scsi/sr.c b/drivers/scsi/sr.c
index 40d8592..93cbd36 100644
--- a/drivers/scsi/sr.c
+++ b/drivers/scsi/sr.c
@@ -79,6 +79,7 @@ MODULE_ALIAS_SCSI_DEVICE(TYPE_WORM);
 static DEFINE_MUTEX(sr_mutex);
 static int sr_probe(struct device *);
 static int sr_remove(struct device *);
+static int sr_init_command(struct scsi_cmnd *SCpnt);
 static int sr_done(struct scsi_cmnd *);
 static int sr_runtime_suspend(struct device *dev);
 
@@ -94,6 +95,7 @@ static struct scsi_driver sr_template = {
 		.remove		= sr_remove,
 		.pm		= &sr_pm_ops,
 	},
+	.init_command		= sr_init_command,
 	.done			= sr_done,
 };
 
@@ -378,21 +380,14 @@ static int sr_done(struct scsi_cmnd *SCpnt)
 	return good_bytes;
 }
 
-static int sr_prep_fn(struct request_queue *q, struct request *rq)
+static int sr_init_command(struct scsi_cmnd *SCpnt)
 {
 	int block = 0, this_count, s_size;
 	struct scsi_cd *cd;
-	struct scsi_cmnd *SCpnt;
-	struct scsi_device *sdp = q->queuedata;
+	struct request *rq = SCpnt->request;
+	struct scsi_device *sdp = SCpnt->device;
 	int ret;
 
-	if (rq->cmd_type == REQ_TYPE_BLOCK_PC) {
-		ret = scsi_setup_blk_pc_cmnd(sdp, rq);
-		goto out;
-	} else if (rq->cmd_type != REQ_TYPE_FS) {
-		ret = BLKPREP_KILL;
-		goto out;
-	}
 	ret = scsi_setup_fs_cmnd(sdp, rq);
 	if (ret != BLKPREP_OK)
 		goto out;
@@ -517,7 +512,7 @@ static int sr_prep_fn(struct request_queue *q, struct request *rq)
 	 */
 	ret = BLKPREP_OK;
  out:
-	return scsi_prep_return(q, rq, ret);
+	return ret;
 }
 
 static int sr_block_open(struct block_device *bdev, fmode_t mode)
@@ -718,7 +713,6 @@ static int sr_probe(struct device *dev)
 
 	/* FIXME: need to handle a get_capabilities failure properly ?? */
 	get_capabilities(cd);
-	blk_queue_prep_rq(sdev->request_queue, sr_prep_fn);
 	sr_vendor_init(cd);
 
 	disk->driverfs_dev = &sdev->sdev_gendev;
@@ -993,7 +987,6 @@ static int sr_remove(struct device *dev)
 
 	scsi_autopm_get_device(cd->device);
 
-	blk_queue_prep_rq(cd->device->request_queue, scsi_prep_fn);
 	del_gendisk(cd->disk);
 
 	mutex_lock(&sr_ref_mutex);
diff --git a/include/scsi/scsi_driver.h b/include/scsi/scsi_driver.h
index 20fdfc2..36c4114 100644
--- a/include/scsi/scsi_driver.h
+++ b/include/scsi/scsi_driver.h
@@ -4,17 +4,17 @@
 #include <linux/device.h>
 
 struct module;
+struct request;
 struct scsi_cmnd;
 struct scsi_device;
-struct request;
-struct request_queue;
-
 
 struct scsi_driver {
 	struct module		*owner;
 	struct device_driver	gendrv;
 
 	void (*rescan)(struct device *);
+	int (*init_command)(struct scsi_cmnd *);
+	void (*uninit_command)(struct scsi_cmnd *);
 	int (*done)(struct scsi_cmnd *);
 	int (*eh_action)(struct scsi_cmnd *, int);
 };
@@ -31,8 +31,5 @@ extern int scsi_register_interface(struct class_interface *);
 
 int scsi_setup_blk_pc_cmnd(struct scsi_device *sdev, struct request *req);
 int scsi_setup_fs_cmnd(struct scsi_device *sdev, struct request *req);
-int scsi_prep_state_check(struct scsi_device *sdev, struct request *req);
-int scsi_prep_return(struct request_queue *q, struct request *req, int ret);
-int scsi_prep_fn(struct request_queue *, struct request *);
 
 #endif /* _SCSI_SCSI_DRIVER_H */
-- 
1.7.10.4


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

* Re: [PATCH 3/4] scsi: reintroduce scsi_driver.init_command
  2014-05-01 14:51 ` [PATCH 3/4] scsi: reintroduce scsi_driver.init_command Christoph Hellwig
@ 2014-05-08  7:11   ` Hannes Reinecke
  0 siblings, 0 replies; 9+ messages in thread
From: Hannes Reinecke @ 2014-05-08  7:11 UTC (permalink / raw)
  To: Christoph Hellwig, James Bottomley; +Cc: linux-scsi

On 05/01/2014 04:51 PM, Christoph Hellwig wrote:
> Instead of letting the ULD play games with the prep_fn move back to
> the model of a central prep_fn with a callback to the ULD.  This
> already cleans up and shortens the code by itself, and will be required
> to properly support blk-mq in the SCSI midlayer.
>

... And leaving the device handler to still provide a prep_fn().
But that's for another day.

> Signed-off-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Nicholas Bellinger <nab@linux-iscsi.org>
> Reviewed-by: Mike Christie <michaelc@cs.wisc.edu>

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

end of thread, other threads:[~2014-05-08  7:11 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-31 10:45 [PATCH 3/4] scsi: reintroduce scsi_driver.init_command Christoph Hellwig
  -- strict thread matches above, loose matches on Subject: below --
2014-05-01 14:51 scsi midlayer updates for 3.15 (resend) Christoph Hellwig
2014-05-01 14:51 ` [PATCH 3/4] scsi: reintroduce scsi_driver.init_command Christoph Hellwig
2014-05-08  7:11   ` Hannes Reinecke
2014-03-27 16:14 misc scsi midlayer updates Christoph Hellwig
2014-03-27 16:14 ` [PATCH 3/4] scsi: reintroduce scsi_driver.init_command Christoph Hellwig
2014-03-31  5:45   ` Nicholas A. Bellinger
2014-03-31  6:08     ` Christoph Hellwig
2014-03-31  6:20       ` Nicholas A. Bellinger
2014-03-31  6:56   ` Mike Christie
2014-03-31  8:09     ` Christoph Hellwig

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.