2022-09-19 07:41:27

by Yu Kuai

Subject: [PATCH v2 3/3] blk-wbt: don't enable throttling if default elevator is bfq

From: Yu Kuai <[email protected]>

Commit b5dc5d4d1f4f ("block,bfq: Disable writeback throttling") tries to
disable wbt for bfq, it's done by calling wbt_disable_default() in
bfq_init_queue(). However, wbt is still enabled if default elevator is
bfq:

device_add_disk
elevator_init_mq
bfq_init_queue
wbt_disable_default -> done nothing

blk_register_queue
wbt_enable_default -> wbt is enabled

Fix the problem by checking elevator name if wbt_enable_default() is
called from blk_register_queue().

Signed-off-by: Yu Kuai <[email protected]>
---
block/bfq-iosched.c | 2 +-
block/blk-sysfs.c | 2 +-
block/blk-wbt.c | 6 +++++-
block/blk-wbt.h | 5 +++--
4 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 7ea427817f7f..f769c90744fd 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -7045,7 +7045,7 @@ static void bfq_exit_queue(struct elevator_queue *e)
#endif

blk_stat_disable_accounting(bfqd->queue);
- wbt_enable_default(bfqd->queue);
+ wbt_enable_default(bfqd->queue, false);

kfree(bfqd);
}
diff --git a/block/blk-sysfs.c b/block/blk-sysfs.c
index 1955bb6a284d..3e8adb95ff02 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -850,7 +850,7 @@ int blk_register_queue(struct gendisk *disk)
goto put_dev;

blk_queue_flag_set(QUEUE_FLAG_REGISTERED, q);
- wbt_enable_default(q);
+ wbt_enable_default(q, true);
blk_throtl_register_queue(q);

/* Now everything is ready and send out KOBJ_ADD uevent */
diff --git a/block/blk-wbt.c b/block/blk-wbt.c
index 68851c2c02d2..c6256e7c9e8e 100644
--- a/block/blk-wbt.c
+++ b/block/blk-wbt.c
@@ -27,6 +27,7 @@

#include "blk-wbt.h"
#include "blk-rq-qos.h"
+#include "elevator.h"

#define CREATE_TRACE_POINTS
#include <trace/events/wbt.h>
@@ -643,10 +644,13 @@ void wbt_set_write_cache(struct request_queue *q, bool write_cache_on)
/*
* Enable wbt if defaults are configured that way
*/
-void wbt_enable_default(struct request_queue *q)
+void wbt_enable_default(struct request_queue *q, bool check_elevator)
{
struct rq_qos *rqos = wbt_rq_qos(q);

+ if (check_elevator && check_elevator_name(q->elevator, "bfq"))
+ return;
+
/* Throttling already enabled? */
if (rqos) {
if (RQWB(rqos)->enable_state == WBT_STATE_OFF_DEFAULT)
diff --git a/block/blk-wbt.h b/block/blk-wbt.h
index e42465ddcbb6..eb028febaff0 100644
--- a/block/blk-wbt.h
+++ b/block/blk-wbt.h
@@ -90,7 +90,7 @@ static inline unsigned int wbt_inflight(struct rq_wb *rwb)

int wbt_init(struct request_queue *);
void wbt_disable_default(struct request_queue *);
-void wbt_enable_default(struct request_queue *);
+void wbt_enable_default(struct request_queue *, bool);

u64 wbt_get_min_lat(struct request_queue *q);
void wbt_set_min_lat(struct request_queue *q, u64 val);
@@ -109,7 +109,8 @@ static inline int wbt_init(struct request_queue *q)
static inline void wbt_disable_default(struct request_queue *q)
{
}
-static inline void wbt_enable_default(struct request_queue *q)
+static inline void wbt_enable_default(struct request_queue *q,
+ bool check_elevator)
{
}
static inline void wbt_set_write_cache(struct request_queue *q, bool wc)
--
2.31.1

2022-09-20 07:48:19

by Christoph Hellwig

Subject: Re: [PATCH v2 3/3] blk-wbt: don't enable throttling if default elevator is bfq

Matching names is always is a bad idea.

Just ad a flags field to elevator_mq_ops, add a DISABLE_WBT flag
to it, and check for that and everything should become much, much
simpler.

2022-09-20 08:16:55

by Yu Kuai

Subject: Re: [PATCH v2 3/3] blk-wbt: don't enable throttling if default elevator is bfq

Hi, Christoph!

?? 2022/09/20 15:25, Christoph Hellwig д??:
> Matching names is always is a bad idea.
>
> Just ad a flags field to elevator_mq_ops, add a DISABLE_WBT flag
> to it, and check for that and everything should become much, much
> simpler.

Thanks for the adivce, that is a good idea!

Kuai
>
> .
>