Re: [PATCH 3/9] bfq: calculate shallow depths at init time

[Omar Sandoval]

On Thu, May 10, 2018 at 10:24:21AM -0600, Jens Axboe wrote:
> It doesn't change, so don't put it in the per-IO hot path.
> 
> Acked-by: Paolo Valente 

Reviewed-by: Omar Sandoval 

> Signed-off-by: Jens Axboe 
> ---
>  block/bfq-iosched.c | 97 
> +++--
>  1 file changed, 50 insertions(+), 47 deletions(-)
> 
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index db38e88a5670..0cd8aa80c32d 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -487,46 +487,6 @@ static struct request *bfq_choose_req(struct bfq_data 
> *bfqd,
>  }
>  
>  /*
> - * See the comments on bfq_limit_depth for the purpose of
> - * the depths set in the function.
> - */
> -static void bfq_update_depths(struct bfq_data *bfqd, struct sbitmap_queue 
> *bt)
> -{
> - bfqd->sb_shift = bt->sb.shift;
> -
> - /*
> -  * In-word depths if no bfq_queue is being weight-raised:
> -  * leaving 25% of tags only for sync reads.
> -  *
> -  * In next formulas, right-shift the value
> -  * (1U -  * (1U<<(bfqd->sb_shift - something)), to be robust against
> -  * any possible value of bfqd->sb_shift, without having to
> -  * limit 'something'.
> -  */
> - /* no more than 50% of tags for async I/O */
> - bfqd->word_depths[0][0] = max((1U>1, 1U);
> - /*
> -  * no more than 75% of tags for sync writes (25% extra tags
> -  * w.r.t. async I/O, to prevent async I/O from starving sync
> -  * writes)
> -  */
> - bfqd->word_depths[0][1] = max(((1U>2, 1U);
> -
> - /*
> -  * In-word depths in case some bfq_queue is being weight-
> -  * raised: leaving ~63% of tags for sync reads. This is the
> -  * highest percentage for which, in our tests, application
> -  * start-up times didn't suffer from any regression due to tag
> -  * shortage.
> -  */
> - /* no more than ~18% of tags for async I/O */
> - bfqd->word_depths[1][0] = max(((1U>4, 1U);
> - /* no more than ~37% of tags for sync writes (~20% extra tags) */
> - bfqd->word_depths[1][1] = max(((1U>4, 1U);
> -}
> -
> -/*
>   * Async I/O can easily starve sync I/O (both sync reads and sync
>   * writes), by consuming all tags. Similarly, storms of sync writes,
>   * such as those that sync(2) may trigger, can starve sync reads.
> @@ -535,18 +495,11 @@ static void bfq_update_depths(struct bfq_data *bfqd, 
> struct sbitmap_queue *bt)
>   */
>  static void bfq_limit_depth(unsigned int op, struct blk_mq_alloc_data *data)
>  {
> - struct blk_mq_tags *tags = blk_mq_tags_from_data(data);
>   struct bfq_data *bfqd = data->q->elevator->elevator_data;
> - struct sbitmap_queue *bt;
>  
>   if (op_is_sync(op) && !op_is_write(op))
>   return;
>  
> - bt = >bitmap_tags;
> -
> - if (unlikely(bfqd->sb_shift != bt->sb.shift))
> - bfq_update_depths(bfqd, bt);
> -
>   data->shallow_depth =
>   bfqd->word_depths[!!bfqd->wr_busy_queues][op_is_sync(op)];
>  
> @@ -5126,6 +5079,55 @@ void bfq_put_async_queues(struct bfq_data *bfqd, 
> struct bfq_group *bfqg)
>   __bfq_put_async_bfqq(bfqd, >async_idle_bfqq);
>  }
>  
> +/*
> + * See the comments on bfq_limit_depth for the purpose of
> + * the depths set in the function.
> + */
> +static void bfq_update_depths(struct bfq_data *bfqd, struct sbitmap_queue 
> *bt)
> +{
> + bfqd->sb_shift = bt->sb.shift;
> +
> + /*
> +  * In-word depths if no bfq_queue is being weight-raised:
> +  * leaving 25% of tags only for sync reads.
> +  *
> +  * In next formulas, right-shift the value
> +  * (1U +  * (1U<<(bfqd->sb_shift - something)), to be robust against
> +  * any possible value of bfqd->sb_shift, without having to
> +  * limit 'something'.
> +  */
> + /* no more than 50% of tags for async I/O */
> + bfqd->word_depths[0][0] = max((1U>1, 1U);
> + /*
> +  * no more than 75% of tags for sync writes (25% extra tags
> +  * w.r.t. async I/O, to prevent async I/O from starving sync
> +  * writes)
> +  */
> + bfqd->word_depths[0][1] = max(((1U>2, 1U);
> +
> + /*
> +  * In-word depths in case some bfq_queue is being weight-
> +  * raised: leaving ~63% of tags for sync reads. This is the
> +  * highest percentage for which, in our tests, application
> +  * start-up times didn't suffer from any regression due to tag
> +  * shortage.
> +  */
> + /* no more than ~18% of tags for async I/O */
> + bfqd->word_depths[1][0] = max(((1U>4, 1U);
> + /* no more than ~37% of tags for sync writes (~20% extra tags) */
> + bfqd->word_depths[1][1] = max(((1U>4, 1U);
> +}
> +
> +static int 

[PATCH 3/9] bfq: calculate shallow depths at init time

[Jens Axboe]

It doesn't change, so don't put it in the per-IO hot path.

Acked-by: Paolo Valente 
Signed-off-by: Jens Axboe 
---
 block/bfq-iosched.c | 97 +++--
 1 file changed, 50 insertions(+), 47 deletions(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index db38e88a5670..0cd8aa80c32d 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -487,46 +487,6 @@ static struct request *bfq_choose_req(struct bfq_data 
*bfqd,
 }
 
 /*
- * See the comments on bfq_limit_depth for the purpose of
- * the depths set in the function.
- */
-static void bfq_update_depths(struct bfq_data *bfqd, struct sbitmap_queue *bt)
-{
-   bfqd->sb_shift = bt->sb.shift;
-
-   /*
-* In-word depths if no bfq_queue is being weight-raised:
-* leaving 25% of tags only for sync reads.
-*
-* In next formulas, right-shift the value
-* (1Usb_shift - something)), to be robust against
-* any possible value of bfqd->sb_shift, without having to
-* limit 'something'.
-*/
-   /* no more than 50% of tags for async I/O */
-   bfqd->word_depths[0][0] = max((1U>1, 1U);
-   /*
-* no more than 75% of tags for sync writes (25% extra tags
-* w.r.t. async I/O, to prevent async I/O from starving sync
-* writes)
-*/
-   bfqd->word_depths[0][1] = max(((1U>2, 1U);
-
-   /*
-* In-word depths in case some bfq_queue is being weight-
-* raised: leaving ~63% of tags for sync reads. This is the
-* highest percentage for which, in our tests, application
-* start-up times didn't suffer from any regression due to tag
-* shortage.
-*/
-   /* no more than ~18% of tags for async I/O */
-   bfqd->word_depths[1][0] = max(((1U>4, 1U);
-   /* no more than ~37% of tags for sync writes (~20% extra tags) */
-   bfqd->word_depths[1][1] = max(((1U>4, 1U);
-}
-
-/*
  * Async I/O can easily starve sync I/O (both sync reads and sync
  * writes), by consuming all tags. Similarly, storms of sync writes,
  * such as those that sync(2) may trigger, can starve sync reads.
@@ -535,18 +495,11 @@ static void bfq_update_depths(struct bfq_data *bfqd, 
struct sbitmap_queue *bt)
  */
 static void bfq_limit_depth(unsigned int op, struct blk_mq_alloc_data *data)
 {
-   struct blk_mq_tags *tags = blk_mq_tags_from_data(data);
struct bfq_data *bfqd = data->q->elevator->elevator_data;
-   struct sbitmap_queue *bt;
 
if (op_is_sync(op) && !op_is_write(op))
return;
 
-   bt = >bitmap_tags;
-
-   if (unlikely(bfqd->sb_shift != bt->sb.shift))
-   bfq_update_depths(bfqd, bt);
-
data->shallow_depth =
bfqd->word_depths[!!bfqd->wr_busy_queues][op_is_sync(op)];
 
@@ -5126,6 +5079,55 @@ void bfq_put_async_queues(struct bfq_data *bfqd, struct 
bfq_group *bfqg)
__bfq_put_async_bfqq(bfqd, >async_idle_bfqq);
 }
 
+/*
+ * See the comments on bfq_limit_depth for the purpose of
+ * the depths set in the function.
+ */
+static void bfq_update_depths(struct bfq_data *bfqd, struct sbitmap_queue *bt)
+{
+   bfqd->sb_shift = bt->sb.shift;
+
+   /*
+* In-word depths if no bfq_queue is being weight-raised:
+* leaving 25% of tags only for sync reads.
+*
+* In next formulas, right-shift the value
+* (1Usb_shift - something)), to be robust against
+* any possible value of bfqd->sb_shift, without having to
+* limit 'something'.
+*/
+   /* no more than 50% of tags for async I/O */
+   bfqd->word_depths[0][0] = max((1U>1, 1U);
+   /*
+* no more than 75% of tags for sync writes (25% extra tags
+* w.r.t. async I/O, to prevent async I/O from starving sync
+* writes)
+*/
+   bfqd->word_depths[0][1] = max(((1U>2, 1U);
+
+   /*
+* In-word depths in case some bfq_queue is being weight-
+* raised: leaving ~63% of tags for sync reads. This is the
+* highest percentage for which, in our tests, application
+* start-up times didn't suffer from any regression due to tag
+* shortage.
+*/
+   /* no more than ~18% of tags for async I/O */
+   bfqd->word_depths[1][0] = max(((1U>4, 1U);
+   /* no more than ~37% of tags for sync writes (~20% extra tags) */
+   bfqd->word_depths[1][1] = max(((1U>4, 1U);
+}
+
+static int bfq_init_hctx(struct blk_mq_hw_ctx *hctx, unsigned int index)
+{
+   struct bfq_data *bfqd = hctx->queue->elevator->elevator_data;
+   struct blk_mq_tags *tags = hctx->sched_tags;
+
+   bfq_update_depths(bfqd, >bitmap_tags);