On 2022-09-13 16:20, Yu Kuai wrote:
在 2022/09/13 22:12, Holger Hoffstätte 写道:
On 2022-09-13 15:39, Yu Kuai wrote:
Hi, Holger
在 2022/08/18 1:12, Holger Hoffstätte 写道:
I just noticed that my device configured with BFQ still shows wbt_lat_usec
as configured, despite the fact that BFQ disables WBT in bfq_init_queue [1]:
$cat /sys/block/sdc/queue/scheduler
mq-deadline [bfq] none
$cat /sys/block/sdc/queue/wbt_lat_usec
75000
Is this supposed to be 0 (since it's disabled) or is sysfs confused?
Thanks,
Holger
I'm reviewing wbt codes recently, and I found that this problem will
happen if the default elevator is bfq. I'll try to fix this, do you mind
if I add reported-by tag?
Do not mind at all - thank you for looking into it!
Let me know if I can test a patch or help in some other way.
Btw not sure what "default scheduler" means here, I set my schedulers via
udev rules. In this case:
ACTION=="add", KERNEL=="sd[a-z]", ATTR{queue/rotational}=="1", ATTR{queue/scheduler}="bfq"
Default means the elevator is bfq when device is created.
Perhaps can you try the following patch?
blk-wbt: don't enable throttling if default elevator is bfq
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().
diff --git a/block/elevator.h b/block/elevator.h
index 3f0593b3bf9d..ccded343cf27 100644
--- a/block/elevator.h
+++ b/block/elevator.h
@@ -104,6 +104,11 @@ struct elevator_queue
DECLARE_HASHTABLE(hash, ELV_HASH_BITS);
};
+static inline bool check_elevator_name(struct elevator_queue *elevator,
+ const char *name)
+{
+ return !strcmp(elevator->type->elevator_name, name);
+}
/*
* block elevator interface
*/
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 e1f009aba6fd..a630d657c054 100644
--- a/block/blk-sysfs.c
+++ b/block/blk-sysfs.c
@@ -843,7 +843,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 246467926253..26ee6ca66a93 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>
@@ -636,10 +637,14 @@ 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 && q->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 7e44eccc676d..1a49b6ac397c 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);
@@ -108,7 +108,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)
So that didn't help, unfortunately.
Directly after boot (with the above udev rule):
$cat /sys/block/sdc/queue/scheduler
mq-deadline [bfq] none
$cat /sys/block/sdc/queue/wbt_lat_usec
75000
Changing the scheduler back and forth also does not help:
$echo mq-deadline > /sys/block/sdc/queue/scheduler
$echo bfq > /sys/block/sdc/queue/scheduler
$cat /sys/block/sdc/queue/wbt_lat_usec
75000
Sorry :)
Holger
