public inbox for linux-block@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] block,bfq: Disable writeback throttling
@ 2017-09-08  9:50 Luca Miccio
  0 siblings, 0 replies; 4+ messages in thread
From: Luca Miccio @ 2017-09-08  9:50 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Paolo Valente, linux-block, linux-kernel, 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

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] block,bfq: Disable writeback throttling
@ 2017-09-14  7:14 oleksandr
  2017-09-19  7:03 ` Paolo Valente
  0 siblings, 1 reply; 4+ messages in thread
From: oleksandr @ 2017-09-14  7:14 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Paolo Valente, linux-block, linux-kernel, Luca Miccio

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] block,bfq: Disable writeback throttling
  2017-09-14  7:14 [PATCH] block,bfq: Disable writeback throttling oleksandr
@ 2017-09-19  7:03 ` Paolo Valente
  0 siblings, 0 replies; 4+ messages in thread
From: Paolo Valente @ 2017-09-19  7:03 UTC (permalink / raw)
  To: oleksandr; +Cc: Jens Axboe, linux-block, Linux Kernel Mailing List, Luca Miccio


> 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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] block,bfq: Disable writeback throttling
       [not found] <20170913100356.3649-1-lucmiccio@gmail.com>
@ 2017-10-03 16:10 ` lee.tibbert
  0 siblings, 0 replies; 4+ messages in thread
From: lee.tibbert @ 2017-10-03 16:10 UTC (permalink / raw)
  To: linux-block; +Cc: lucmiccio, paolo.valente, 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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-10-03 16:10 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-14  7:14 [PATCH] block,bfq: Disable writeback throttling oleksandr
2017-09-19  7:03 ` Paolo Valente
     [not found] <20170913100356.3649-1-lucmiccio@gmail.com>
2017-10-03 16:10 ` lee.tibbert
  -- strict thread matches above, loose matches on Subject: below --
2017-09-08  9:50 Luca Miccio

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox