diff mbox series

[-next,v2] block: remove init_mutex and open-code blk_iolatency_try_init

Message ID 20230809112928.2009183-1-lilingfeng@huaweicloud.com (mailing list archive)
State New, archived
Headers show
Series [-next,v2] block: remove init_mutex and open-code blk_iolatency_try_init | expand

Commit Message

Li Lingfeng Aug. 9, 2023, 11:29 a.m. UTC 
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.

Since init_mutex has been removed, blk_iolatency_try_init can be
open-coded back to iolatency_set_limit() like ioc_qos_write().

Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com>
---
  v1->v2: open-code blk_iolatency_try_init()

 block/blk-iolatency.c | 34 ++++++++++------------------------
 1 file changed, 10 insertions(+), 24 deletions(-)

Comments

Michal Koutný Aug. 9, 2023, 12:11 p.m. UTC | #1 
On Wed, Aug 09, 2023 at 07:29:28PM +0800, Li Lingfeng <lilingfeng@huaweicloud.com> wrote:
>  1 file changed, 10 insertions(+), 24 deletions(-)

I like this bottom line.

> @@ -861,7 +838,16 @@ static ssize_t iolatency_set_limit(struct kernfs_open_file *of, char *buf,
>  
>  	blkg_conf_init(&ctx, buf);
>  
> -	ret = blk_iolatency_try_init(&ctx);
> +	ret = blkg_conf_open_bdev(&ctx);
> +	if (ret)
> +		goto out;
> +
> +	/*
> +	 * blk_iolatency_init() may fail after rq_qos_add() succeeds which can
> +	 * confuse iolat_rq_qos() test. Make the test and init atomic.
> +	 */

Perhaps add here
	lockdep_assert_held(ctx.bdev->bd_queue->rq_qos_mutex);

because without that the last sentence of the comment misses the
context.

> +	if (!iolat_rq_qos(ctx.bdev->bd_queue))
> +		ret = blk_iolatency_init(ctx.bdev->bd_disk);
>  	if (ret)
>  		goto out;

Thanks,
Michal
Li Lingfeng Aug. 10, 2023, 3:20 a.m. UTC | #2 
在 2023/8/9 20:11, Michal Koutný 写道:
> On Wed, Aug 09, 2023 at 07:29:28PM +0800, Li Lingfeng <lilingfeng@huaweicloud.com> wrote:
>>   1 file changed, 10 insertions(+), 24 deletions(-)
> I like this bottom line.
>
>> @@ -861,7 +838,16 @@ static ssize_t iolatency_set_limit(struct kernfs_open_file *of, char *buf,
>>   
>>   	blkg_conf_init(&ctx, buf);
>>   
>> -	ret = blk_iolatency_try_init(&ctx);
>> +	ret = blkg_conf_open_bdev(&ctx);
>> +	if (ret)
>> +		goto out;
>> +
>> +	/*
>> +	 * blk_iolatency_init() may fail after rq_qos_add() succeeds which can
>> +	 * confuse iolat_rq_qos() test. Make the test and init atomic.
>> +	 */
> Perhaps add here
> 	lockdep_assert_held(ctx.bdev->bd_queue->rq_qos_mutex);
>
> because without that the last sentence of the comment misses the
> context.

Yes, it looks better.

Thanks.

Li

>
>> +	if (!iolat_rq_qos(ctx.bdev->bd_queue))
>> +		ret = blk_iolatency_init(ctx.bdev->bd_disk);
>>   	if (ret)
>>   		goto out;
> Thanks,
> Michal
diff mbox series

Patch

diff --git a/block/blk-iolatency.c b/block/blk-iolatency.c
index fd5fec989e39..e1dbf18a1d1d 100644
--- a/block/blk-iolatency.c
+++ b/block/blk-iolatency.c
@@ -824,29 +824,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);
-	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);
-
-	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)
 {
@@ -861,7 +838,16 @@  static ssize_t iolatency_set_limit(struct kernfs_open_file *of, char *buf,
 
 	blkg_conf_init(&ctx, buf);
 
-	ret = blk_iolatency_try_init(&ctx);
+	ret = blkg_conf_open_bdev(&ctx);
+	if (ret)
+		goto out;
+
+	/*
+	 * blk_iolatency_init() may fail after rq_qos_add() succeeds which can
+	 * confuse iolat_rq_qos() test. Make the test and init atomic.
+	 */
+	if (!iolat_rq_qos(ctx.bdev->bd_queue))
+		ret = blk_iolatency_init(ctx.bdev->bd_disk);
 	if (ret)
 		goto out;