Re: [PATCH] block: BFQ default for single queue devices
Hi! > I talked to Pavel a bit back and it turns out he has a > usecase for BFQ as well and I bet he also would like it > as default scheduler for that system (Pavel tell us more, > I don't remember what it was!) I'm not sure I remember clearly, either. IIRC I was working with ionice on spinning disks, and it had no effect. I switched to BFQ and suddenly ionice was effective. Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [PATCH] block: BFQ default for single queue devices
> Il giorno 02 ott 2018, alle ore 14:43, Linus Walleij
> ha scritto:
>
> This sets BFQ as the default scheduler for single queue
> block devices (nr_hw_queues == 1) if it is available. This
> affects notably MMC/SD-cards but notably also UBI and
> the loopback device.
>
> I have been running it for a while without any negative
> effects on my pet systems and I want some wider testing
> so let's throw it out there and see what people say.
> Admittedly my use cases are limited.
>
> I talked to Pavel a bit back and it turns out he has a
> usecase for BFQ as well and I bet he also would like it
> as default scheduler for that system (Pavel tell us more,
> I don't remember what it was!)
>
> Intuitively I could understand that maybe we want to
> leave the loop device
Actually, I've tested loop devices too. And, also with these virtual
devices, switching to bfq radically improves figures of merits as
responsiveness and latency for soft real-time applications.
Thanks,
Paolo
> (possibly others? nbd? rbd?) as
> "none", as it is probably relying on a scheduler on the
> device below it, so I'm open to passing in a scheduler hint
> from the respective subsystem in say struct blk_mq_tag_set.
> However that makes for a bit of syntactic dissonance
> with the struct member ".nr_hw_queues" (I wonder how
> the loop device can have 1 "hardware queue"?) so
> maybe we should in that case also rename that struct
> member to ".nr_queues" fair and square before we start
> making adjustments for treating queues differently whether
> they are in hardware or actually not.
>
> Cc: Pavel Machek
> Cc: Paolo Valente
> Cc: Jens Axboe
> Cc: Ulf Hansson
> Cc: Richard Weinberger
> Cc: Artem Bityutskiy
> Cc: Adrian Hunter
> Signed-off-by: Linus Walleij
> ---
> block/elevator.c | 21 ++---
> 1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/block/elevator.c b/block/elevator.c
> index e18ac68626e3..e5a2c39eee7b 100644
> --- a/block/elevator.c
> +++ b/block/elevator.c
> @@ -948,13 +948,15 @@ int elevator_switch_mq(struct request_queue *q,
> }
>
> /*
> - * For blk-mq devices, we default to using mq-deadline, if available, for
> single
> - * queue devices. If deadline isn't available OR we have multiple queues,
> - * default to "none".
> + * For blk-mq devices, we default to using:
> + * - "none" for multiqueue devices (nr_hw_queues != 1)
> + * - "bfq", if available, for single queue devices
> + * - "mq-deadline" if "bfq" is not available for single queue devices
> + * - "none" for single queue devices as well as last resort
> */
> int elevator_init_mq(struct request_queue *q)
> {
> - struct elevator_type *e;
> + struct elevator_type *e = NULL;
> int err = 0;
>
> if (q->nr_hw_queues != 1)
> @@ -968,9 +970,14 @@ int elevator_init_mq(struct request_queue *q)
> if (unlikely(q->elevator))
> goto out_unlock;
>
> - e = elevator_get(q, "mq-deadline", false);
> - if (!e)
> - goto out_unlock;
> + if (IS_ENABLED(CONFIG_IOSCHED_BFQ))
> + e = elevator_get(q, "bfq", false);
> +
> + if (!e) {
> + e = elevator_get(q, "mq-deadline", false);
> + if (!e)
> + goto out_unlock;
> + }
>
> err = blk_mq_init_sched(q, e);
> if (err)
> --
> 2.17.1
>
Re: [PATCH] block: BFQ default for single queue devices
Linus, Am Dienstag, 2. Oktober 2018, 14:43:29 CEST schrieb Linus Walleij: > This sets BFQ as the default scheduler for single queue > block devices (nr_hw_queues == 1) if it is available. This > affects notably MMC/SD-cards but notably also UBI and > the loopback device. did you notice a difference for UBI? Strictly speaking it affects only ubibock, the read-only block device on top of an UBI volume. Thanks, //richard
Re: [PATCH] block: BFQ default for single queue devices
On Tue, Oct 2, 2018 at 4:31 PM Jens Axboe wrote: > On 10/2/18 6:43 AM, Linus Walleij wrote: > > This sets BFQ as the default scheduler for single queue > > block devices (nr_hw_queues == 1) if it is available. This > > affects notably MMC/SD-cards but notably also UBI and > > the loopback device. > > I think this should just be done with udev rules, and I'd > prefer if the distros would lead the way on this, as they > are the ones that will most likely see the most bug reports > on a change like this. AFAICT there is no sysfs property that states how many hw queues the device has. And what we want to do is activate BFQ when there is one HW queue. Should I make a patch to add a nr_hw_queues sysfs file for this purpose in that case? That will be a slightly misleading file for loop or networked devices. udev is a way to do this with desktop/server distros that has "standard" (as they think about it) userspace. They can even do it from their initrd/initramfs to mount root using BFQ I guess (quick handover from e.g. UEFI). However this is not a very good fit with Embedded systems, as they tend to be minimal, not use udev (e.g. Android, OpenWRT, busybox-derivates...) they don't do udev rules, but I guess they can in theory do other scripts. But they will mount root before anything like that can happen. They don't use initrd/initramfs. What I want to achieve is to mount my rootfs with BFQ but that is not possible on embedded systems that do not use initramfs, e.g. a rootfs on MMC/SD or UBI. Yours, Linus Walleij
Re: [PATCH] block: BFQ default for single queue devices
On 10/2/18 6:43 AM, Linus Walleij wrote: > This sets BFQ as the default scheduler for single queue > block devices (nr_hw_queues == 1) if it is available. This > affects notably MMC/SD-cards but notably also UBI and > the loopback device. > > I have been running it for a while without any negative > effects on my pet systems and I want some wider testing > so let's throw it out there and see what people say. > Admittedly my use cases are limited. > > I talked to Pavel a bit back and it turns out he has a > usecase for BFQ as well and I bet he also would like it > as default scheduler for that system (Pavel tell us more, > I don't remember what it was!) > > Intuitively I could understand that maybe we want to > leave the loop device (possibly others? nbd? rbd?) as > "none", as it is probably relying on a scheduler on the > device below it, so I'm open to passing in a scheduler hint > from the respective subsystem in say struct blk_mq_tag_set. > However that makes for a bit of syntactic dissonance > with the struct member ".nr_hw_queues" (I wonder how > the loop device can have 1 "hardware queue"?) so > maybe we should in that case also rename that struct > member to ".nr_queues" fair and square before we start > making adjustments for treating queues differently whether > they are in hardware or actually not. I think this should just be done with udev rules, and I'd prefer if the distros would lead the way on this, as they are the ones that will most likely see the most bug reports on a change like this. -- Jens Axboe
[PATCH] block: BFQ default for single queue devices
This sets BFQ as the default scheduler for single queue
block devices (nr_hw_queues == 1) if it is available. This
affects notably MMC/SD-cards but notably also UBI and
the loopback device.
I have been running it for a while without any negative
effects on my pet systems and I want some wider testing
so let's throw it out there and see what people say.
Admittedly my use cases are limited.
I talked to Pavel a bit back and it turns out he has a
usecase for BFQ as well and I bet he also would like it
as default scheduler for that system (Pavel tell us more,
I don't remember what it was!)
Intuitively I could understand that maybe we want to
leave the loop device (possibly others? nbd? rbd?) as
"none", as it is probably relying on a scheduler on the
device below it, so I'm open to passing in a scheduler hint
from the respective subsystem in say struct blk_mq_tag_set.
However that makes for a bit of syntactic dissonance
with the struct member ".nr_hw_queues" (I wonder how
the loop device can have 1 "hardware queue"?) so
maybe we should in that case also rename that struct
member to ".nr_queues" fair and square before we start
making adjustments for treating queues differently whether
they are in hardware or actually not.
Cc: Pavel Machek
Cc: Paolo Valente
Cc: Jens Axboe
Cc: Ulf Hansson
Cc: Richard Weinberger
Cc: Artem Bityutskiy
Cc: Adrian Hunter
Signed-off-by: Linus Walleij
---
block/elevator.c | 21 ++---
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/block/elevator.c b/block/elevator.c
index e18ac68626e3..e5a2c39eee7b 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -948,13 +948,15 @@ int elevator_switch_mq(struct request_queue *q,
}
/*
- * For blk-mq devices, we default to using mq-deadline, if available, for
single
- * queue devices. If deadline isn't available OR we have multiple queues,
- * default to "none".
+ * For blk-mq devices, we default to using:
+ * - "none" for multiqueue devices (nr_hw_queues != 1)
+ * - "bfq", if available, for single queue devices
+ * - "mq-deadline" if "bfq" is not available for single queue devices
+ * - "none" for single queue devices as well as last resort
*/
int elevator_init_mq(struct request_queue *q)
{
- struct elevator_type *e;
+ struct elevator_type *e = NULL;
int err = 0;
if (q->nr_hw_queues != 1)
@@ -968,9 +970,14 @@ int elevator_init_mq(struct request_queue *q)
if (unlikely(q->elevator))
goto out_unlock;
- e = elevator_get(q, "mq-deadline", false);
- if (!e)
- goto out_unlock;
+ if (IS_ENABLED(CONFIG_IOSCHED_BFQ))
+ e = elevator_get(q, "bfq", false);
+
+ if (!e) {
+ e = elevator_get(q, "mq-deadline", false);
+ if (!e)
+ goto out_unlock;
+ }
err = blk_mq_init_sched(q, e);
if (err)
--
2.17.1
