Re: wbt_lat_usec still set despite wbt disabled by BFQ

[Yu Kuai <yukuai1@huaweicloud.com>]

在 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)

> cheers
> Holger
> .
>