Re: [PATCH] block,bfq: Disable writeback throttling

Re: [PATCH] block,bfq: Disable writeback throttling

[lee.tibbert]

Luca,

Please feel free to add:

  Tested-by: Lee Tibbert <lee.tibbert@gmail.com>

The short story is that, with one small commenting issue, the patch is
fine on my system.  The small issue is that block/blk-wbt.c line 657 says:

    Disable wbt, if enabled by default. Only called from CFQ.

As of this patch, that comment is no longer true.  Adding "& BFQ" is
the obvious edit, but simply deleting the "Only" sentence is probably
more robust. IMHO, that should not hold up acceptence of this patch.

Longer story:

      In addition to running this for almost three weeks now, I explicitly
      tested a number of paths CONFIG_BLK_WBT enabled &
      disabled, etc. Details upon request.

      In particular, I tested that when CONFIG_BLK_WBT is enabled
      sys/block/sda/queue/wbt_lat_usec is cleared when the elevator
      is set to BFQ(BFQ-MQ). Patch does what it purports.

      I also tested that wbt_lat_usec gets set back to its default when
      an elevator other than BFQ is selected after BFQ is active. (Yeah,
      the new elevator probably does this. There is no code in BFQ to do it).

      This patch makes it easier to get write back settings right when using
      cgroups.  With cgroup, on wants  CONFIG_BLK_DEV_THROTTLING on
      but CONFIG_BLK_WBT off. This patch painlessly assures the latter
      and reduces, at least my, confusion.

Thank you for this patch. I hope that it is accepted for linux-block in
as time allows.

Lee

Re: [PATCH] block,bfq: Disable writeback throttling

[oleksandr]

Tested-by: Oleksandr Natalenko <oleksandr@natalenko.name>

> Similarly to CFQ, BFQ has its write-throttling heuristics, and it
> is better not to combine them with further write-throttling
> heuristics of a different nature.
> So this commit disables write-back throttling for a device if BFQ
> is used as I/O scheduler for that device.
> 
> Signed-off-by: Luca Miccio <lucmiccio@gmail.com>
> Signed-off-by: Paolo Valente <paolo.valente@linaro.org>
> ---
>  block/bfq-iosched.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Re: [PATCH] block,bfq: Disable writeback throttling

[Paolo Valente]

> Il giorno 14 set 2017, alle ore 09:14, oleksandr@natalenko.name ha scritto:
> 
> Tested-by: Oleksandr Natalenko <oleksandr@natalenko.name>
> 
>> Similarly to CFQ, BFQ has its write-throttling heuristics, and it
>> is better not to combine them with further write-throttling
>> heuristics of a different nature.
>> So this commit disables write-back throttling for a device if BFQ
>> is used as I/O scheduler for that device.
>> Signed-off-by: Luca Miccio <lucmiccio@gmail.com>
>> Signed-off-by: Paolo Valente <paolo.valente@linaro.org>
>> ---
>> block/bfq-iosched.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)

Hi Jens,
did you have time to look into this?

Thanks,
Paolo

[PATCH] block,bfq: Disable writeback throttling

[Luca Miccio]

Similarly to CFQ, BFQ has its write-throttling heuristics, and it
is better not to combine them with further write-throttling
heuristics of a different nature.
So this commit disables write-back throttling for a device if BFQ
is used as I/O scheduler for that device.

Signed-off-by: Luca Miccio <lucmiccio@gmail.com>
Signed-off-by: Paolo Valente <paolo.valente@linaro.org>
---
 block/bfq-iosched.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 436b6ca..4feae52 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -108,6 +108,7 @@
 #include "blk-mq-tag.h"
 #include "blk-mq-sched.h"
 #include "bfq-iosched.h"
+#include "blk-wbt.h"
 
 #define BFQ_BFQQ_FNS(name)						\
 void bfq_mark_bfqq_##name(struct bfq_queue *bfqq)			\
@@ -4760,7 +4761,7 @@ static int bfq_init_queue(struct request_queue *q, struct elevator_type *e)
 	bfq_init_root_group(bfqd->root_group, bfqd);
 	bfq_init_entity(&bfqd->oom_bfqq.entity, bfqd->root_group);
 
-
+	wbt_disable_default(q);
 	return 0;
 
 out_free:
-- 
2.7.4