| Message ID | 20230804113659.3816877-1-lilingfeng@huaweicloud.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [-next] block: remove init_mutex in blk_iolatency_try_init | expand |
在 2023/08/04 19:36, Li Lingfeng 写道: > From: Li Lingfeng <lilingfeng3@huawei.com> > > Commit a13696b83da4 ("blk-iolatency: Make initialization lazy") adds > a mutex named "init_mutex" in blk_iolatency_try_init for the race > condition of initializing RQ_QOS_LATENCY. > Now a new lock has been add to struct request_queue by commit a13bd91be223 > ("block/rq_qos: protect rq_qos apis with a new lock"). And it has been > held in blkg_conf_open_bdev before calling blk_iolatency_init. > So it's not necessary to keep init_mutex in blk_iolatency_try_init, just > remove it. LGTM Reviewed-by: Yu Kuai <yukuai3@huawei.com> > > Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com> > --- > block/blk-iolatency.c | 5 ----- > 1 file changed, 5 deletions(-) > > diff --git a/block/blk-iolatency.c b/block/blk-iolatency.c > index fd5fec989e39..8fbd6bc96acb 100644 > --- a/block/blk-iolatency.c > +++ b/block/blk-iolatency.c > @@ -826,7 +826,6 @@ 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); > @@ -837,13 +836,9 @@ static int blk_iolatency_try_init(struct blkg_conf_ctx *ctx) > * 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); > - > if (!iolat_rq_qos(ctx->bdev->bd_queue)) > ret = blk_iolatency_init(ctx->bdev->bd_disk); > > - mutex_unlock(&init_mutex); > - > return ret; > } > >
Hello. On Fri, Aug 04, 2023 at 07:36:59PM +0800, Li Lingfeng <lilingfeng@huaweicloud.com> wrote: > From: Li Lingfeng <lilingfeng3@huawei.com> > > Commit a13696b83da4 ("blk-iolatency: Make initialization lazy") adds > a mutex named "init_mutex" in blk_iolatency_try_init for the race > condition of initializing RQ_QOS_LATENCY. > Now a new lock has been add to struct request_queue by commit a13bd91be223 > ("block/rq_qos: protect rq_qos apis with a new lock"). And it has been > held in blkg_conf_open_bdev before calling blk_iolatency_init. > So it's not necessary to keep init_mutex in blk_iolatency_try_init, just > remove it. I'm looking at ioc_cost_model_write() or ioc_qos_write() where is a similar pattern. Could the check+init be open-coded back to iolatency_set_limit() to make code more regular? Michal
Thanks for your advice, I will send a new patch soon. 在 2023/8/8 0:05, Michal Koutný 写道: > Hello. > > On Fri, Aug 04, 2023 at 07:36:59PM +0800, Li Lingfeng <lilingfeng@huaweicloud.com> wrote: >> From: Li Lingfeng <lilingfeng3@huawei.com> >> >> Commit a13696b83da4 ("blk-iolatency: Make initialization lazy") adds >> a mutex named "init_mutex" in blk_iolatency_try_init for the race >> condition of initializing RQ_QOS_LATENCY. >> Now a new lock has been add to struct request_queue by commit a13bd91be223 >> ("block/rq_qos: protect rq_qos apis with a new lock"). And it has been >> held in blkg_conf_open_bdev before calling blk_iolatency_init. >> So it's not necessary to keep init_mutex in blk_iolatency_try_init, just >> remove it. > I'm looking at ioc_cost_model_write() or ioc_qos_write() where is > a similar pattern. > Could the check+init be open-coded back to iolatency_set_limit() to make > code more regular? > > Michal
diff --git a/block/blk-iolatency.c b/block/blk-iolatency.c index fd5fec989e39..8fbd6bc96acb 100644 --- a/block/blk-iolatency.c +++ b/block/blk-iolatency.c @@ -826,7 +826,6 @@ 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); @@ -837,13 +836,9 @@ static int blk_iolatency_try_init(struct blkg_conf_ctx *ctx) * 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); - if (!iolat_rq_qos(ctx->bdev->bd_queue)) ret = blk_iolatency_init(ctx->bdev->bd_disk); - mutex_unlock(&init_mutex); - return ret; }