[PATCH 0/4] blk-mq updates

[PATCH 0/4] blk-mq updates

[Christoph Hellwig]

Two fixes for partial completion support, and the addition of two methods
to allow drivers to easily init/cleanup per-request allocations.

[PATCH 1/4] blk-mq: fix blk_mq_end_io_partial

[Christoph Hellwig]

[-- Attachment #1: 0001-blk-mq-fix-blk_mq_end_io_partial.patch --]
[-- Type: text/plain, Size: 629 bytes --]

We need to pass the actual nr_bytes instead of all bytes in the request.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-mq.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 883f720..d81dc8b 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -285,7 +285,7 @@ void blk_mq_free_request(struct request *rq)
 
 bool blk_mq_end_io_partial(struct request *rq, int error, unsigned int nr_bytes)
 {
-	if (blk_update_request(rq, error, blk_rq_bytes(rq)))
+	if (blk_update_request(rq, error, nr_bytes))
 		return true;
 
 	blk_account_io_done(rq);
-- 
1.7.10.4

[PATCH 2/4] blk-mq: initialize resid_len

[Christoph Hellwig]

[-- Attachment #1: 0002-blk-mq-initialize-resid_len.patch --]
[-- Type: text/plain, Size: 534 bytes --]

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-mq.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index d81dc8b..e3284f6 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -350,6 +350,8 @@ static void blk_mq_start_request(struct request *rq, bool last)
 
 	trace_block_rq_issue(q, rq);
 
+	rq->resid_len = blk_rq_bytes(rq);
+
 	/*
 	 * Just mark start time and set the started bit. Due to memory
 	 * ordering, we know we'll see the correct deadline as long as
-- 
1.7.10.4

[PATCH 3/4] blk-mq: replace blk_mq_init_commands with a ->init_request method

[Christoph Hellwig]

[-- Attachment #1: 0003-blk-mq-replace-blk_mq_init_commands-with-a-init_requ.patch --]
[-- Type: text/plain, Size: 6687 bytes --]

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@lst.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;
+	}
+
 	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

[PATCH 4/4] blk-mq: add a exit_request method

[Christoph Hellwig]

[-- Attachment #1: 0004-blk-mq-add-a-exit_request-method.patch --]
[-- Type: text/plain, Size: 1590 bytes --]

This gives drivers an easy way to free any resources allocated in
->init_request.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 block/blk-mq.c |   18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index c2ce99b..c7e723e 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1000,6 +1000,16 @@ static void blk_mq_free_rq_map(struct blk_mq_hw_ctx *hctx)
 {
 	struct page *page;
 
+	if (hctx->rqs && hctx->queue->mq_ops->exit_request) {
+		int i;
+
+		for (i = 0; i < hctx->queue_depth; i++) {
+			if (!hctx->rqs[i])
+				continue;
+			hctx->queue->mq_ops->exit_request(hctx, hctx->rqs[i]);
+		}
+	}
+
 	while (!list_empty(&hctx->page_list)) {
 		page = list_first_entry(&hctx->page_list, struct page, lru);
 		list_del_init(&page->lru);
@@ -1332,7 +1342,7 @@ struct request_queue *blk_mq_init_queue(struct blk_mq_reg *reg,
 	}
 
 	if (blk_mq_init_hw_queues(q, reg, driver_data))
-		goto err_flush_rq;
+		goto err_flush_rq_init;
 
 	blk_mq_map_swqueue(q);
 
@@ -1342,6 +1352,9 @@ struct request_queue *blk_mq_init_queue(struct blk_mq_reg *reg,
 
 	return q;
 
+err_flush_rq_init:
+	if (reg->ops->exit_request)
+		reg->ops->exit_request(NULL, q->flush_rq);
 err_flush_rq:
 	kfree(q->flush_rq);
 err_hw:
@@ -1387,6 +1400,9 @@ void blk_mq_free_queue(struct request_queue *q)
 	mutex_lock(&all_q_mutex);
 	list_del_init(&q->all_q_node);
 	mutex_unlock(&all_q_mutex);
+
+	if (q->mq_ops->exit_request)
+		q->mq_ops->exit_request(NULL, q->flush_rq);
 }
 
 /* Basically redo blk_mq_init_queue with queue frozen */
-- 
1.7.10.4

Re: [PATCH 3/4] blk-mq: replace blk_mq_init_commands with a ->init_request method

[Ming Lei]

On Mon, Mar 17, 2014 at 9:18 PM, Christoph Hellwig <hch@infradead.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@lst.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@vger.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

Re: [PATCH 4/4] blk-mq: add a exit_request method

[Ming Lei]

On Mon, Mar 17, 2014 at 9:18 PM, Christoph Hellwig <hch@infradead.org> wrote:
> This gives drivers an easy way to free any resources allocated in
> ->init_request.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  block/blk-mq.c |   18 +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index c2ce99b..c7e723e 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -1000,6 +1000,16 @@ static void blk_mq_free_rq_map(struct blk_mq_hw_ctx *hctx)
>  {
>         struct page *page;
>
> +       if (hctx->rqs && hctx->queue->mq_ops->exit_request) {

exit_request definition is missed.


Thanks,
-- 
Ming Lei

Re: [PATCH 3/4] blk-mq: replace blk_mq_init_commands with a ->init_request method

[Christoph Hellwig]

On Wed, Mar 19, 2014 at 11:08:20AM +0800, Ming Lei wrote:
> > +               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.

You mean the blk_rq_init?  We already do a real initialization before
actually using it, it's just there to prevent passing a half-initialized
one to the driver.

Re: [PATCH 4/4] blk-mq: add a exit_request method

[Christoph Hellwig]

On Wed, Mar 19, 2014 at 01:22:07PM +0800, Ming Lei wrote:
> exit_request definition is missed.

oops that accidentally went into another commit.

New version below:

---
From: Christoph Hellwig <hch@infradead.org>
Subject: blk-mq: add a exit_request method

This gives drivers an easy way to free any ressources allocated in
->init_request.

Signed-off-by: Christoph Hellwig <hch@lst.de>

diff --git a/block/blk-mq.c b/block/blk-mq.c
index c2ce99b..c7e723e 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1000,6 +1000,16 @@ static void blk_mq_free_rq_map(struct blk_mq_hw_ctx *hctx)
 {
 	struct page *page;
 
+	if (hctx->rqs && hctx->queue->mq_ops->exit_request) {
+		int i;
+
+		for (i = 0; i < hctx->queue_depth; i++) {
+			if (!hctx->rqs[i])
+				continue;
+			hctx->queue->mq_ops->exit_request(hctx, hctx->rqs[i]);
+		}
+	}
+
 	while (!list_empty(&hctx->page_list)) {
 		page = list_first_entry(&hctx->page_list, struct page, lru);
 		list_del_init(&page->lru);
@@ -1332,7 +1342,7 @@ struct request_queue *blk_mq_init_queue(struct blk_mq_reg *reg,
 	}
 
 	if (blk_mq_init_hw_queues(q, reg, driver_data))
-		goto err_flush_rq;
+		goto err_flush_rq_init;
 
 	blk_mq_map_swqueue(q);
 
@@ -1342,6 +1352,9 @@ struct request_queue *blk_mq_init_queue(struct blk_mq_reg *reg,
 
 	return q;
 
+err_flush_rq_init:
+	if (reg->ops->exit_request)
+		reg->ops->exit_request(NULL, q->flush_rq);
 err_flush_rq:
 	kfree(q->flush_rq);
 err_hw:
@@ -1387,6 +1400,9 @@ void blk_mq_free_queue(struct request_queue *q)
 	mutex_lock(&all_q_mutex);
 	list_del_init(&q->all_q_node);
 	mutex_unlock(&all_q_mutex);
+
+	if (q->mq_ops->exit_request)
+		q->mq_ops->exit_request(NULL, q->flush_rq);
 }
 
 /* Basically redo blk_mq_init_queue with queue frozen */
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 4d34957..ff194f8 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -68,6 +68,7 @@ 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);
+typedef void (exit_request_fn)(struct blk_mq_hw_ctx *, struct request *);
 
 struct blk_mq_ops {
 	/*
@@ -104,8 +105,10 @@ struct blk_mq_ops {
 	/*
 	 * Called for every command allocated by the block layer to allow
 	 * the driver to set up driver specific data.
+	 * Ditto for exit/teardown.
 	 */
 	init_request_fn		*init_request;
+	exit_request_fn		*exit_request;
 };
 
 enum {

Re: [PATCH 3/4] blk-mq: replace blk_mq_init_commands with a ->init_request method

[Ming Lei]

On Wed, Mar 19, 2014 at 3:54 PM, Christoph Hellwig <hch@infradead.org> wrote:
> On Wed, Mar 19, 2014 at 11:08:20AM +0800, Ming Lei wrote:
>> > +               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.
>
> You mean the blk_rq_init?  We already do a real initialization before
> actually using it, it's just there to prevent passing a half-initialized
> one to the driver.
>

OK.

Actually it doesn't matter because flush_rq will be reinitialized
before using, not like common request. And driver should only
initialize req->special instead of the request itself, maybe it is
better to document the fact.


Thanks,
-- 
Ming Lei

Re: [PATCH 3/4] blk-mq: replace blk_mq_init_commands with a ->init_request method

[Christoph Hellwig]

On Wed, Mar 19, 2014 at 04:48:47PM +0800, Ming Lei wrote:
> Actually it doesn't matter because flush_rq will be reinitialized
> before using, not like common request. And driver should only
> initialize req->special instead of the request itself, maybe it is
> better to document the fact.

I'll send a blurb to document this fact with the next round of updates.