[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CACVXFVMsFzyiR5gqyGk6jc0pRZP-3CZSvxr+oi36umVWJPjWZw@mail.gmail.com>
Date: Wed, 19 Mar 2014 11:08:20 +0800
From: Ming Lei <tom.leiming@...il.com>
To: Christoph Hellwig <hch@...radead.org>
Cc: Jens Axboe <axboe@...nel.dk>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/4] blk-mq: replace blk_mq_init_commands with a
->init_request method
On Mon, Mar 17, 2014 at 9:18 PM, Christoph Hellwig <hch@...radead.org> wrote:
> Bedides a simpler and cleared interface this also allows to initialize the
> flush_rq properly. The interface for that is still a bit messy as we don't
> have a hw_ctx available for the flush request, but that's something that
> should be fixable with a little work once the demand arises.
>
> Signed-off-by: Christoph Hellwig <hch@....de>
> ---
> block/blk-mq.c | 65 +++++++++++++++++++++-----------------------
> drivers/block/virtio_blk.c | 22 +++++++--------
> include/linux/blk-mq.h | 9 +++++-
> 3 files changed, 50 insertions(+), 46 deletions(-)
>
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index e3284f6..c2ce99b 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -996,33 +996,6 @@ static void blk_mq_hctx_notify(void *data, unsigned long action,
> blk_mq_put_ctx(ctx);
> }
>
> -static void blk_mq_init_hw_commands(struct blk_mq_hw_ctx *hctx,
> - void (*init)(void *, struct blk_mq_hw_ctx *,
> - struct request *, unsigned int),
> - void *data)
> -{
> - unsigned int i;
> -
> - for (i = 0; i < hctx->queue_depth; i++) {
> - struct request *rq = hctx->rqs[i];
> -
> - init(data, hctx, rq, i);
> - }
> -}
> -
> -void blk_mq_init_commands(struct request_queue *q,
> - void (*init)(void *, struct blk_mq_hw_ctx *,
> - struct request *, unsigned int),
> - void *data)
> -{
> - struct blk_mq_hw_ctx *hctx;
> - unsigned int i;
> -
> - queue_for_each_hw_ctx(q, hctx, i)
> - blk_mq_init_hw_commands(hctx, init, data);
> -}
> -EXPORT_SYMBOL(blk_mq_init_commands);
> -
> static void blk_mq_free_rq_map(struct blk_mq_hw_ctx *hctx)
> {
> struct page *page;
> @@ -1050,10 +1023,12 @@ static size_t order_to_size(unsigned int order)
> }
>
> static int blk_mq_init_rq_map(struct blk_mq_hw_ctx *hctx,
> - unsigned int reserved_tags, int node)
> + struct blk_mq_reg *reg, void *driver_data, int node)
> {
> + unsigned int reserved_tags = reg->reserved_tags;
> unsigned int i, j, entries_per_page, max_order = 4;
> size_t rq_size, left;
> + int error;
>
> INIT_LIST_HEAD(&hctx->page_list);
>
> @@ -1102,14 +1077,23 @@ static int blk_mq_init_rq_map(struct blk_mq_hw_ctx *hctx,
> for (j = 0; j < to_do; j++) {
> hctx->rqs[i] = p;
> blk_mq_rq_init(hctx, hctx->rqs[i]);
> + if (reg->ops->init_request) {
> + error = reg->ops->init_request(driver_data,
> + hctx, hctx->rqs[i], i);
> + if (error)
> + goto err_rq_map;
> + }
> +
> p += rq_size;
> i++;
> }
> }
>
> - if (i < (reserved_tags + BLK_MQ_TAG_MIN))
> + if (i < (reserved_tags + BLK_MQ_TAG_MIN)) {
> + error = -ENOMEM;
> goto err_rq_map;
> - else if (i != hctx->queue_depth) {
> + }
> + if (i != hctx->queue_depth) {
> hctx->queue_depth = i;
> pr_warn("%s: queue depth set to %u because of low memory\n",
> __func__, i);
> @@ -1117,12 +1101,14 @@ static int blk_mq_init_rq_map(struct blk_mq_hw_ctx *hctx,
>
> hctx->tags = blk_mq_init_tags(hctx->queue_depth, reserved_tags, node);
> if (!hctx->tags) {
> -err_rq_map:
> - blk_mq_free_rq_map(hctx);
> - return -ENOMEM;
> + error = -ENOMEM;
> + goto err_rq_map;
> }
>
> return 0;
> +err_rq_map:
> + blk_mq_free_rq_map(hctx);
> + return error;
> }
>
> static int blk_mq_init_hw_queues(struct request_queue *q,
> @@ -1155,7 +1141,7 @@ static int blk_mq_init_hw_queues(struct request_queue *q,
> blk_mq_hctx_notify, hctx);
> blk_mq_register_cpu_notifier(&hctx->cpu_notifier);
>
> - if (blk_mq_init_rq_map(hctx, reg->reserved_tags, node))
> + if (blk_mq_init_rq_map(hctx, reg, driver_data, node))
> break;
>
> /*
> @@ -1334,6 +1320,17 @@ struct request_queue *blk_mq_init_queue(struct blk_mq_reg *reg,
> if (!q->flush_rq)
> goto err_hw;
>
> + if (reg->ops->init_request) {
> + blk_rq_init(q, q->flush_rq);
> + if (reg->cmd_size)
> + q->flush_rq->special =
> + blk_mq_rq_to_pdu(q->flush_rq);
> +
> + if (reg->ops->init_request(driver_data,
> + NULL, q->flush_rq, -1))
> + goto err_flush_rq;
> + }
The above looks a bit weird because q->flush_rq is invisible to
driver and should always be initialized no matter if driver defines
its .init_request callback or not.
> +
> if (blk_mq_init_hw_queues(q, reg, driver_data))
> goto err_flush_rq;
>
> diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> index b1cb3f4..a2e925b 100644
> --- a/drivers/block/virtio_blk.c
> +++ b/drivers/block/virtio_blk.c
> @@ -474,11 +474,22 @@ static const struct device_attribute dev_attr_cache_type_rw =
> __ATTR(cache_type, S_IRUGO|S_IWUSR,
> virtblk_cache_type_show, virtblk_cache_type_store);
>
> +static int virtblk_init_request(void *data, struct blk_mq_hw_ctx *hctx,
> + struct request *rq, unsigned int nr)
> +{
> + struct virtio_blk *vblk = data;
> + struct virtblk_req *vbr = rq->special;
> +
> + sg_init_table(vbr->sg, vblk->sg_elems);
> + return 0;
> +}
> +
> static struct blk_mq_ops virtio_mq_ops = {
> .queue_rq = virtio_queue_rq,
> .map_queue = blk_mq_map_queue,
> .alloc_hctx = blk_mq_alloc_single_hw_queue,
> .free_hctx = blk_mq_free_single_hw_queue,
> + .init_request = virtblk_init_request,
> .complete = virtblk_request_done,
> };
>
> @@ -490,15 +501,6 @@ static struct blk_mq_reg virtio_mq_reg = {
> .flags = BLK_MQ_F_SHOULD_MERGE,
> };
>
> -static void virtblk_init_vbr(void *data, struct blk_mq_hw_ctx *hctx,
> - struct request *rq, unsigned int nr)
> -{
> - struct virtio_blk *vblk = data;
> - struct virtblk_req *vbr = rq->special;
> -
> - sg_init_table(vbr->sg, vblk->sg_elems);
> -}
> -
> static int virtblk_probe(struct virtio_device *vdev)
> {
> struct virtio_blk *vblk;
> @@ -562,8 +564,6 @@ static int virtblk_probe(struct virtio_device *vdev)
> goto out_put_disk;
> }
>
> - blk_mq_init_commands(q, virtblk_init_vbr, vblk);
> -
> q->queuedata = vblk;
>
> virtblk_name_format("vd", index, vblk->disk->disk_name, DISK_NAME_LEN);
> diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
> index 2ff2e8d..cac10e1 100644
> --- a/include/linux/blk-mq.h
> +++ b/include/linux/blk-mq.h
> @@ -66,6 +66,8 @@ typedef struct blk_mq_hw_ctx *(alloc_hctx_fn)(struct blk_mq_reg *,unsigned int);
> typedef void (free_hctx_fn)(struct blk_mq_hw_ctx *, unsigned int);
> typedef int (init_hctx_fn)(struct blk_mq_hw_ctx *, void *, unsigned int);
> typedef void (exit_hctx_fn)(struct blk_mq_hw_ctx *, unsigned int);
> +typedef int (init_request_fn)(void *, struct blk_mq_hw_ctx *,
> + struct request *, unsigned int);
>
> struct blk_mq_ops {
> /*
> @@ -98,6 +100,12 @@ struct blk_mq_ops {
> */
> init_hctx_fn *init_hctx;
> exit_hctx_fn *exit_hctx;
> +
> + /*
> + * Called for every command allocated by the block layer to allow
> + * the driver to set up driver specific data.
> + */
> + init_request_fn *init_request;
> };
>
> enum {
> @@ -117,7 +125,6 @@ enum {
> struct request_queue *blk_mq_init_queue(struct blk_mq_reg *, void *);
> int blk_mq_register_disk(struct gendisk *);
> void blk_mq_unregister_disk(struct gendisk *);
> -void blk_mq_init_commands(struct request_queue *, void (*init)(void *data, struct blk_mq_hw_ctx *, struct request *, unsigned int), void *data);
>
> void blk_mq_flush_plug_list(struct blk_plug *plug, bool from_schedule);
>
> --
> 1.7.10.4
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
Thanks,
--
Ming Lei
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists