lkml.org 
[lkml]   [2023]   [Apr]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 4/4] blk-iolatency: Make initialization lazy
From
Date
Hi,

在 2023/04/13 8:06, Tejun Heo 写道:
> Other rq_qos policies such as wbt and iocost are lazy-initialized when they
> are configured for the first time for the device but iolatency is
> initialized unconditionally from blkcg_init_disk() during gendisk init. Lazy
> init is beneficial because rq_qos policies add runtime overhead when
> initialized as every IO has to walk all registered rq_qos callbacks.
>
> This patch switches iolatency to lazy initialization too so that it only
> registered its rq_qos policy when it is first configured.
>
> Note that there is a known race condition between blkcg config file writes
> and del_gendisk() and this patch makes iolatency susceptible to it by
> exposing the init path to race against the deletion path. However, that
> problem already exists in iocost and is being worked on.
>
> Signed-off-by: Tejun Heo <tj@kernel.org>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Cc: Josef Bacik <josef@toxicpanda.com>
> ---
> block/blk-cgroup.c | 8 --------
> block/blk-iolatency.c | 29 ++++++++++++++++++++++++++++-
> block/blk.h | 6 ------
> 3 files changed, 28 insertions(+), 15 deletions(-)
>
> diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
> index c154b08a7e92..1c1ebeb51003 100644
> --- a/block/blk-cgroup.c
> +++ b/block/blk-cgroup.c
> @@ -33,7 +33,6 @@
> #include "blk-cgroup.h"
> #include "blk-ioprio.h"
> #include "blk-throttle.h"
> -#include "blk-rq-qos.h"
>
> /*
> * blkcg_pol_mutex protects blkcg_policy[] and policy [de]activation.
> @@ -1350,14 +1349,8 @@ int blkcg_init_disk(struct gendisk *disk)
> if (ret)
> goto err_ioprio_exit;
>
> - ret = blk_iolatency_init(disk);
> - if (ret)
> - goto err_throtl_exit;
> -
> return 0;
>
> -err_throtl_exit:
> - blk_throtl_exit(disk);
> err_ioprio_exit:
> blk_ioprio_exit(disk);
> err_destroy_all:
> @@ -1373,7 +1366,6 @@ int blkcg_init_disk(struct gendisk *disk)
> void blkcg_exit_disk(struct gendisk *disk)
> {
> blkg_destroy_all(disk);
> - rq_qos_exit(disk->queue);
> blk_throtl_exit(disk);
> }
>
> diff --git a/block/blk-iolatency.c b/block/blk-iolatency.c
> index 2560708b9109..fd5fec989e39 100644
> --- a/block/blk-iolatency.c
> +++ b/block/blk-iolatency.c
> @@ -755,7 +755,7 @@ static void blkiolatency_enable_work_fn(struct work_struct *work)
> }
> }
>
> -int blk_iolatency_init(struct gendisk *disk)
> +static int blk_iolatency_init(struct gendisk *disk)
> {
> struct blk_iolatency *blkiolat;
> int ret;
> @@ -824,6 +824,29 @@ static void iolatency_clear_scaling(struct blkcg_gq *blkg)
> }
> }
>
> +static int blk_iolatency_try_init(struct blkg_conf_ctx *ctx)
> +{
> + static DEFINE_MUTEX(init_mutex);
> + int ret;
> +
> + ret = blkg_conf_open_bdev(ctx);
> + if (ret)
> + return ret;
> +
> + /*
> + * blk_iolatency_init() may fail after rq_qos_add() succeeds which can
> + * confuse iolat_rq_qos() test. Make the test and init atomic.
> + */
> + mutex_lock(&init_mutex);

Can we just convert the return value from -EBUSY to 0 in this case, so
that the global lock won't be needed.

Thanks,
Kuai
> +
> + if (!iolat_rq_qos(ctx->bdev->bd_queue))
> + ret = blk_iolatency_init(ctx->bdev->bd_disk);
> +
> + mutex_unlock(&init_mutex);
> +
> + return ret;
> +}
> +
> static ssize_t iolatency_set_limit(struct kernfs_open_file *of, char *buf,
> size_t nbytes, loff_t off)
> {
> @@ -838,6 +861,10 @@ static ssize_t iolatency_set_limit(struct kernfs_open_file *of, char *buf,
>
> blkg_conf_init(&ctx, buf);
>
> + ret = blk_iolatency_try_init(&ctx);
> + if (ret)
> + goto out;
> +
> ret = blkg_conf_prep(blkcg, &blkcg_policy_iolatency, &ctx);
> if (ret)
> goto out;
> diff --git a/block/blk.h b/block/blk.h
> index d65d96994a94..62fca868bc61 100644
> --- a/block/blk.h
> +++ b/block/blk.h
> @@ -399,12 +399,6 @@ static inline struct bio *blk_queue_bounce(struct bio *bio,
> return bio;
> }
>
> -#ifdef CONFIG_BLK_CGROUP_IOLATENCY
> -int blk_iolatency_init(struct gendisk *disk);
> -#else
> -static inline int blk_iolatency_init(struct gendisk *disk) { return 0; };
> -#endif
> -
> #ifdef CONFIG_BLK_DEV_ZONED
> void disk_free_zone_bitmaps(struct gendisk *disk);
> void disk_clear_zone_settings(struct gendisk *disk);
>

\
 
 \ /
  Last update: 2023-04-13 04:10    [W:0.091 / U:0.176 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site