Re: [PATCH 06/10] blk-mq-tag: cleanup the normal/reserved tag allocation

[Jens Axboe]

On Fri, Jan 13 2017, Christoph Hellwig wrote:
> On Wed, Jan 11, 2017 at 02:39:59PM -0700, Jens Axboe wrote:
> > This is in preparation for having another tag set available. Cleanup
> > the parameters, and allow passing in of tags fo blk_mq_put_tag().
> 
> I've been playing around with this are a bit but never submitted
> anything.  Below is an untested merge of my previous bits with your
> patch, let me know what you think about it:

Looks good to me. I'll test it and merge it with the other patch, if
successful.

-- 
Jens Axboe

Re: [PATCH 06/10] blk-mq-tag: cleanup the normal/reserved tag allocation

[Jens Axboe]

On Fri, Jan 13 2017, Christoph Hellwig wrote:
> On Wed, Jan 11, 2017 at 02:39:59PM -0700, Jens Axboe wrote:
> > This is in preparation for having another tag set available. Cleanup
> > the parameters, and allow passing in of tags fo blk_mq_put_tag().
> 
> I've been playing around with this are a bit but never submitted
> anything.  Below is an untested merge of my previous bits with your
> patch, let me know what you think about it:

Looks good to me. I'll test it and merge it with the other patch, if
successful.

-- 
Jens Axboe

Re: [PATCH 06/10] blk-mq-tag: cleanup the normal/reserved tag allocation

[Christoph Hellwig]

On Wed, Jan 11, 2017 at 02:39:59PM -0700, Jens Axboe wrote:
> This is in preparation for having another tag set available. Cleanup
> the parameters, and allow passing in of tags fo blk_mq_put_tag().

I've been playing around with this are a bit but never submitted
anything.  Below is an untested merge of my previous bits with your
patch, let me know what you think about it:

---
>From c9ea92bfb468e9116149db95d246e48ace2b87f1 Mon Sep 17 00:00:00 2001
From: Jens Axboe 
Date: Wed, 11 Jan 2017 11:04:53 -0700
Subject: blk-mq-tag: cleanup the normal/reserved tag allocation

This is in preparation for having another tag set available. Cleanup
the parameters, and allow passing in of tags for blk_mq_put_tag().

Signed-off-by: Jens Axboe 
[hch: even more cleanups]
Signed-off-by: Christoph Hellwig 
---
 block/blk-mq-tag.c | 94 +-
 block/blk-mq-tag.h |  4 +--
 block/blk-mq.c |  2 +-
 block/blk-mq.h |  5 +++
 4 files changed, 44 insertions(+), 61 deletions(-)

diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index dcf5ce3..ced7527 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -90,32 +90,46 @@ static inline bool hctx_may_queue(struct blk_mq_hw_ctx 
*hctx,
return atomic_read(>nr_active) < depth;
 }
 
-static int __bt_get(struct blk_mq_hw_ctx *hctx, struct sbitmap_queue *bt)
+static int __blk_mq_get_tag(struct blk_mq_hw_ctx *hctx, struct sbitmap_queue 
*bt)
 {
if (!hctx_may_queue(hctx, bt))
return -1;
return __sbitmap_queue_get(bt);
 }
 
-static int bt_get(struct blk_mq_alloc_data *data, struct sbitmap_queue *bt,
- struct blk_mq_hw_ctx *hctx, struct blk_mq_tags *tags)
+unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data)
 {
+   struct blk_mq_tags *tags = blk_mq_tags_from_data(data);
+   struct sbitmap_queue *bt;
struct sbq_wait_state *ws;
DEFINE_WAIT(wait);
+   unsigned int tag_offset;
int tag;
 
-   tag = __bt_get(hctx, bt);
+   if (data->flags & BLK_MQ_REQ_RESERVED) {
+   if (unlikely(!tags->nr_reserved_tags)) {
+   WARN_ON_ONCE(1);
+   return BLK_MQ_TAG_FAIL;
+   }
+   bt = >breserved_tags;
+   tag_offset = 0;
+   } else {
+   bt = >bitmap_tags;
+   tag_offset = tags->nr_reserved_tags;
+   }
+
+   tag = __blk_mq_get_tag(data->hctx, bt);
if (tag != -1)
-   return tag;
+   goto found_tag;
 
if (data->flags & BLK_MQ_REQ_NOWAIT)
-   return -1;
+   return BLK_MQ_TAG_FAIL;
 
-   ws = bt_wait_ptr(bt, hctx);
+   ws = bt_wait_ptr(bt, data->hctx);
do {
prepare_to_wait(>wait, , TASK_UNINTERRUPTIBLE);
 
-   tag = __bt_get(hctx, bt);
+   tag = __blk_mq_get_tag(data->hctx, bt);
if (tag != -1)
break;
 
@@ -125,14 +139,14 @@ static int bt_get(struct blk_mq_alloc_data *data, struct 
sbitmap_queue *bt,
 * some to complete. Note that hctx can be NULL here for
 * reserved tag allocation.
 */
-   if (hctx)
-   blk_mq_run_hw_queue(hctx, false);
+   if (data->hctx)
+   blk_mq_run_hw_queue(data->hctx, false);
 
/*
 * Retry tag allocation after running the hardware queue,
 * as running the queue may also have found completions.
 */
-   tag = __bt_get(hctx, bt);
+   tag = __blk_mq_get_tag(data->hctx, bt);
if (tag != -1)
break;
 
@@ -142,61 +156,25 @@ static int bt_get(struct blk_mq_alloc_data *data, struct 
sbitmap_queue *bt,
 
data->ctx = blk_mq_get_ctx(data->q);
data->hctx = blk_mq_map_queue(data->q, data->ctx->cpu);
-   if (data->flags & BLK_MQ_REQ_RESERVED) {
-   bt = >hctx->tags->breserved_tags;
-   } else {
-   hctx = data->hctx;
-   bt = >tags->bitmap_tags;
-   }
+   tags = blk_mq_tags_from_data(data);
+   if (data->flags & BLK_MQ_REQ_RESERVED)
+   bt = >breserved_tags;
+   else
+   bt = >bitmap_tags;
+
finish_wait(>wait, );
-   ws = bt_wait_ptr(bt, hctx);
+   ws = bt_wait_ptr(bt, data->hctx);
} while (1);
 
finish_wait(>wait, );
-   return tag;
-}
-
-static unsigned int __blk_mq_get_tag(struct blk_mq_alloc_data *data)
-{
-   int tag;
-
-   tag = bt_get(data, >hctx->tags->bitmap_tags, data->hctx,
-data->hctx->tags);
-   if (tag >= 0)
-   return tag + data->hctx->tags->nr_reserved_tags;
-
-   

Re: [PATCH 06/10] blk-mq-tag: cleanup the normal/reserved tag allocation

[Christoph Hellwig]

On Wed, Jan 11, 2017 at 02:39:59PM -0700, Jens Axboe wrote:
> This is in preparation for having another tag set available. Cleanup
> the parameters, and allow passing in of tags fo blk_mq_put_tag().

I've been playing around with this are a bit but never submitted
anything.  Below is an untested merge of my previous bits with your
patch, let me know what you think about it:

---
>From c9ea92bfb468e9116149db95d246e48ace2b87f1 Mon Sep 17 00:00:00 2001
From: Jens Axboe 
Date: Wed, 11 Jan 2017 11:04:53 -0700
Subject: blk-mq-tag: cleanup the normal/reserved tag allocation

This is in preparation for having another tag set available. Cleanup
the parameters, and allow passing in of tags for blk_mq_put_tag().

Signed-off-by: Jens Axboe 
[hch: even more cleanups]
Signed-off-by: Christoph Hellwig 
---
 block/blk-mq-tag.c | 94 +-
 block/blk-mq-tag.h |  4 +--
 block/blk-mq.c |  2 +-
 block/blk-mq.h |  5 +++
 4 files changed, 44 insertions(+), 61 deletions(-)

diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c
index dcf5ce3..ced7527 100644
--- a/block/blk-mq-tag.c
+++ b/block/blk-mq-tag.c
@@ -90,32 +90,46 @@ static inline bool hctx_may_queue(struct blk_mq_hw_ctx 
*hctx,
return atomic_read(>nr_active) < depth;
 }
 
-static int __bt_get(struct blk_mq_hw_ctx *hctx, struct sbitmap_queue *bt)
+static int __blk_mq_get_tag(struct blk_mq_hw_ctx *hctx, struct sbitmap_queue 
*bt)
 {
if (!hctx_may_queue(hctx, bt))
return -1;
return __sbitmap_queue_get(bt);
 }
 
-static int bt_get(struct blk_mq_alloc_data *data, struct sbitmap_queue *bt,
- struct blk_mq_hw_ctx *hctx, struct blk_mq_tags *tags)
+unsigned int blk_mq_get_tag(struct blk_mq_alloc_data *data)
 {
+   struct blk_mq_tags *tags = blk_mq_tags_from_data(data);
+   struct sbitmap_queue *bt;
struct sbq_wait_state *ws;
DEFINE_WAIT(wait);
+   unsigned int tag_offset;
int tag;
 
-   tag = __bt_get(hctx, bt);
+   if (data->flags & BLK_MQ_REQ_RESERVED) {
+   if (unlikely(!tags->nr_reserved_tags)) {
+   WARN_ON_ONCE(1);
+   return BLK_MQ_TAG_FAIL;
+   }
+   bt = >breserved_tags;
+   tag_offset = 0;
+   } else {
+   bt = >bitmap_tags;
+   tag_offset = tags->nr_reserved_tags;
+   }
+
+   tag = __blk_mq_get_tag(data->hctx, bt);
if (tag != -1)
-   return tag;
+   goto found_tag;
 
if (data->flags & BLK_MQ_REQ_NOWAIT)
-   return -1;
+   return BLK_MQ_TAG_FAIL;
 
-   ws = bt_wait_ptr(bt, hctx);
+   ws = bt_wait_ptr(bt, data->hctx);
do {
prepare_to_wait(>wait, , TASK_UNINTERRUPTIBLE);
 
-   tag = __bt_get(hctx, bt);
+   tag = __blk_mq_get_tag(data->hctx, bt);
if (tag != -1)
break;
 
@@ -125,14 +139,14 @@ static int bt_get(struct blk_mq_alloc_data *data, struct 
sbitmap_queue *bt,
 * some to complete. Note that hctx can be NULL here for
 * reserved tag allocation.
 */
-   if (hctx)
-   blk_mq_run_hw_queue(hctx, false);
+   if (data->hctx)
+   blk_mq_run_hw_queue(data->hctx, false);
 
/*
 * Retry tag allocation after running the hardware queue,
 * as running the queue may also have found completions.
 */
-   tag = __bt_get(hctx, bt);
+   tag = __blk_mq_get_tag(data->hctx, bt);
if (tag != -1)
break;
 
@@ -142,61 +156,25 @@ static int bt_get(struct blk_mq_alloc_data *data, struct 
sbitmap_queue *bt,
 
data->ctx = blk_mq_get_ctx(data->q);
data->hctx = blk_mq_map_queue(data->q, data->ctx->cpu);
-   if (data->flags & BLK_MQ_REQ_RESERVED) {
-   bt = >hctx->tags->breserved_tags;
-   } else {
-   hctx = data->hctx;
-   bt = >tags->bitmap_tags;
-   }
+   tags = blk_mq_tags_from_data(data);
+   if (data->flags & BLK_MQ_REQ_RESERVED)
+   bt = >breserved_tags;
+   else
+   bt = >bitmap_tags;
+
finish_wait(>wait, );
-   ws = bt_wait_ptr(bt, hctx);
+   ws = bt_wait_ptr(bt, data->hctx);
} while (1);
 
finish_wait(>wait, );
-   return tag;
-}
-
-static unsigned int __blk_mq_get_tag(struct blk_mq_alloc_data *data)
-{
-   int tag;
-
-   tag = bt_get(data, >hctx->tags->bitmap_tags, data->hctx,
-data->hctx->tags);
-   if (tag >= 0)
-   return tag + data->hctx->tags->nr_reserved_tags;
-
-   return BLK_MQ_TAG_FAIL;
-}
-
-static 

Re: [PATCH 06/10] blk-mq-tag: cleanup the normal/reserved tag allocation

[Jens Axboe]

On Thu, Jan 12 2017, Bart Van Assche wrote:
> On Wed, 2017-01-11 at 14:39 -0700, Jens Axboe wrote:
> > This is in preparation for having another tag set available. Cleanup
> > the parameters, and allow passing in of tags fo blk_mq_put_tag().
> 
> It seems like an 'r' is missing from the description ("tags fo")?

Indeed, good eye. Added.

-- 
Jens Axboe

Re: [PATCH 06/10] blk-mq-tag: cleanup the normal/reserved tag allocation

[Jens Axboe]

On Thu, Jan 12 2017, Bart Van Assche wrote:
> On Wed, 2017-01-11 at 14:39 -0700, Jens Axboe wrote:
> > This is in preparation for having another tag set available. Cleanup
> > the parameters, and allow passing in of tags fo blk_mq_put_tag().
> 
> It seems like an 'r' is missing from the description ("tags fo")?

Indeed, good eye. Added.

-- 
Jens Axboe

Re: [PATCH 06/10] blk-mq-tag: cleanup the normal/reserved tag allocation

[Bart Van Assche]

On Wed, 2017-01-11 at 14:39 -0700, Jens Axboe wrote:
> This is in preparation for having another tag set available. Cleanup
> the parameters, and allow passing in of tags fo blk_mq_put_tag().

It seems like an 'r' is missing from the description ("tags fo")?

Anyway:

Reviewed-by: Bart Van Assche 

Re: [PATCH 06/10] blk-mq-tag: cleanup the normal/reserved tag allocation

[Bart Van Assche]

On Wed, 2017-01-11 at 14:39 -0700, Jens Axboe wrote:
> This is in preparation for having another tag set available. Cleanup
> the parameters, and allow passing in of tags fo blk_mq_put_tag().

It seems like an 'r' is missing from the description ("tags fo")?

Anyway:

Reviewed-by: Bart Van Assche