From: Christoph Hellwig <hch@infradead.org>
To: linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 28/39] scsi: reintroduce scsi_driver.init_command
Date: Wed, 26 Mar 2014 02:46:54 -0700 [thread overview]
Message-ID: <20140326094654.GB30655@infradead.org> (raw)
In-Reply-To: <20140317133135.514350463@bombadil.infradead.org>
This is another one I'd like to send off to James ASAP, can I get some
reviews?
On Mon, Mar 17, 2014 at 06:28:01AM -0700, 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.
>
> 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 a23e8c3..9889c75 100644
> --- a/drivers/scsi/scsi_lib.c
> +++ b/drivers/scsi/scsi_lib.c
> @@ -470,6 +470,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()
> *
> @@ -494,6 +504,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;
> @@ -1078,15 +1090,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
> @@ -1130,15 +1134,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;
> }
> @@ -1148,16 +1148,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;
>
> @@ -1209,9 +1206,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;
>
> @@ -1242,18 +1239,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 470954a..33e349b 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);
> @@ -2872,9 +2863,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) {
> @@ -3021,8 +3009,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
>
>
> --
> 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
---end quoted text---
next prev parent reply other threads:[~2014-03-26 9:46 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-03-17 13:27 [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
2014-03-17 13:27 ` [PATCH 01/39] block: fix q->flush_rq NULL pointer crash on dm-mpath flush Christoph Hellwig
2014-03-17 13:27 ` [PATCH 02/39] block: change flush sequence list addition back to front add Christoph Hellwig
2014-03-17 13:27 ` [PATCH 03/39] blk-mq: fix blk_mq_end_io_partial Christoph Hellwig
2014-03-17 13:27 ` [PATCH 04/39] blk-mq: initialize resid_len Christoph Hellwig
2014-03-17 13:27 ` [PATCH 05/39] blk-mq: replace blk_mq_init_commands with a ->init_request method Christoph Hellwig
2014-03-17 13:27 ` [PATCH 06/39] blk-mq: add a exit_request method Christoph Hellwig
2014-03-17 13:27 ` [PATCH 07/39] scsi: avoid useless free_list lock roundtrips Christoph Hellwig
2014-03-17 13:27 ` [PATCH 08/39] scsi: avoid taking host_lock in scsi_run_queue unless nessecary Christoph Hellwig
2014-03-17 13:27 ` [PATCH 09/39] scsi: do not manipulate device reference counts in scsi_get/put_command Christoph Hellwig
2014-03-17 13:27 ` [PATCH 10/39] scsi: remove a useless get/put_device pair in scsi_request_fn Christoph Hellwig
2014-03-17 13:27 ` [PATCH 11/39] scsi: remove a useless get/put_device pair in scsi_next_command Christoph Hellwig
2014-03-17 13:27 ` [PATCH 12/39] scsi: remove a useless get/put_device pair in scsi_requeue_command Christoph Hellwig
2014-03-17 13:27 ` [PATCH 13/39] megaraid: simplify internal command handling Christoph Hellwig
2014-03-21 1:12 ` adam radford
2014-03-17 13:27 ` [PATCH 14/39] scsi: simplify command allocation and freeing a bit Christoph Hellwig
2014-03-17 13:27 ` [PATCH 15/39] scsi: add support for per-host cmd pools Christoph Hellwig
2014-03-17 13:27 ` [PATCH 16/39] virtio_scsi: use cmd_size Christoph Hellwig
2014-03-25 15:31 ` Christoph Hellwig
2014-03-25 15:36 ` Paolo Bonzini
2014-03-27 15:28 ` James Bottomley
2014-03-17 13:27 ` [PATCH 17/39] scsi: explicitly release bidi buffers Christoph Hellwig
2014-03-17 13:27 ` [PATCH 18/39] scsi: remove scsi_end_request Christoph Hellwig
2014-03-17 13:27 ` [PATCH 19/39] scsi: push host_lock down into scsi_{host,target}_queue_ready Christoph Hellwig
2014-03-17 13:27 ` [PATCH 20/39] scsi: convert target_busy to an atomic_t Christoph Hellwig
2014-03-17 13:27 ` [PATCH 21/39] scsi: convert host_busy to atomic_t Christoph Hellwig
2014-03-17 13:27 ` [PATCH 22/39] scsi: convert device_busy " Christoph Hellwig
2014-03-17 13:27 ` [PATCH 23/39] scsi: fix the {host,target,device}_blocked counter mess Christoph Hellwig
2014-03-17 13:27 ` [PATCH 24/39] blk-mq: add blk_mq_requeue_request Christoph Hellwig
2014-03-17 13:27 ` [PATCH 25/39] blk-mq: add async paramter to blk_mq_start_stopped_hw_queues Christoph Hellwig
2014-03-17 13:27 ` [PATCH 26/39] HACK: support blk_delay_queue for blk-mq Christoph Hellwig
2014-03-17 13:28 ` [PATCH 27/39] blk-mq: export blk_mq_insert_request Christoph Hellwig
2014-03-17 13:28 ` [PATCH 28/39] scsi: reintroduce scsi_driver.init_command Christoph Hellwig
2014-03-26 9:46 ` Christoph Hellwig [this message]
2014-03-17 13:28 ` [PATCH 29/39] block: remove unprep_rq_fn Christoph Hellwig
2014-03-17 13:28 ` [PATCH 30/39] scsi: centralize command re-queueing in scsi_dispatch_fn Christoph Hellwig
2014-03-17 13:28 ` [PATCH 31/39] scsi: split __scsi_queue_insert Christoph Hellwig
2014-03-17 13:28 ` [PATCH 32/39] scsi: unwind blk_end_request_all and blk_end_request_err calls Christoph Hellwig
2014-03-17 13:28 ` [PATCH 33/39] scsi: initial blk-mq support Christoph Hellwig
2014-03-17 13:28 ` [PATCH 34/39] scsi: partially stub out scsi_adjust_queue_depth when using blk-mq Christoph Hellwig
2014-03-17 13:28 ` [PATCH 35/39] virtio_scsi: use blk_mq Christoph Hellwig
2014-03-17 13:28 ` [PATCH 36/39] iscsi_tcp: " Christoph Hellwig
2014-03-17 13:28 ` [PATCH 37/39] ata_piix: " Christoph Hellwig
2014-03-17 13:28 ` [PATCH 38/39] blk-mq: make blk_mq_start_stopped_hw_queues run a queue even if not stopped Christoph Hellwig
2014-03-17 13:28 ` [PATCH 39/39] scsi: implement ->init_request and ->exit_request Christoph Hellwig
2014-03-17 13:33 ` [PATCH 00/39] [WIP] scsi multiqueue Christoph Hellwig
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140326094654.GB30655@infradead.org \
--to=hch@infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox