2023-01-27 15:44:03

by Ulf Hansson

[permalink] [raw]
Subject: [PATCH] block: Default to build the BFQ I/O scheduler

Today BFQ is widely used and it's also the default choice for some of the
single-queue-based storage devices. Therefore, let's make it more
convenient to build it as default, along with the other I/O schedulers.

Let's also build the cgroup support for BFQ as default, as it's likely that
it's wanted too, assuming CONFIG_BLK_CGROUP is also set, of course.

Signed-off-by: Ulf Hansson <[email protected]>
---
block/Kconfig.iosched | 2 ++
1 file changed, 2 insertions(+)

diff --git a/block/Kconfig.iosched b/block/Kconfig.iosched
index 615516146086..221739143d44 100644
--- a/block/Kconfig.iosched
+++ b/block/Kconfig.iosched
@@ -18,6 +18,7 @@ config MQ_IOSCHED_KYBER

config IOSCHED_BFQ
tristate "BFQ I/O scheduler"
+ default y
select BLK_ICQ
help
BFQ I/O scheduler for BLK-MQ. BFQ distributes the bandwidth of
@@ -30,6 +31,7 @@ config IOSCHED_BFQ
config BFQ_GROUP_IOSCHED
bool "BFQ hierarchical scheduling support"
depends on IOSCHED_BFQ && BLK_CGROUP
+ default y
select BLK_CGROUP_RWSTAT
help

--
2.34.1



2023-01-27 15:48:49

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH] block: Default to build the BFQ I/O scheduler

On 1/27/23 8:43 AM, Ulf Hansson wrote:
> Today BFQ is widely used and it's also the default choice for some of the
> single-queue-based storage devices. Therefore, let's make it more
> convenient to build it as default, along with the other I/O schedulers.
>
> Let's also build the cgroup support for BFQ as default, as it's likely that
> it's wanted too, assuming CONFIG_BLK_CGROUP is also set, of course.

This won't make much of a difference, when the symbols are already in
the .config. So let's please not. It may be a 'y' for you by default,
but for lots of others it is not. Don't impose it on folks.

--
Jens Axboe



2023-01-27 15:58:51

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH] block: Default to build the BFQ I/O scheduler

On Fri, 27 Jan 2023 at 16:48, Jens Axboe <[email protected]> wrote:
>
> On 1/27/23 8:43 AM, Ulf Hansson wrote:
> > Today BFQ is widely used and it's also the default choice for some of the
> > single-queue-based storage devices. Therefore, let's make it more
> > convenient to build it as default, along with the other I/O schedulers.
> >
> > Let's also build the cgroup support for BFQ as default, as it's likely that
> > it's wanted too, assuming CONFIG_BLK_CGROUP is also set, of course.
>
> This won't make much of a difference, when the symbols are already in
> the .config. So let's please not. It may be a 'y' for you by default,
> but for lots of others it is not. Don't impose it on folks.

This isn't about folkz, but HW. :-)

I was thinking that it makes sense for the similar reason to why kyber
and deadline are being built as default. Or are there any particular
other reasons to why we build those in as default, but not BFQ?

Kind regards
Uffe

2023-01-27 20:11:05

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH] block: Default to build the BFQ I/O scheduler

On 1/27/23 8:58 AM, Ulf Hansson wrote:
> On Fri, 27 Jan 2023 at 16:48, Jens Axboe <[email protected]> wrote:
>>
>> On 1/27/23 8:43 AM, Ulf Hansson wrote:
>>> Today BFQ is widely used and it's also the default choice for some of the
>>> single-queue-based storage devices. Therefore, let's make it more
>>> convenient to build it as default, along with the other I/O schedulers.
>>>
>>> Let's also build the cgroup support for BFQ as default, as it's likely that
>>> it's wanted too, assuming CONFIG_BLK_CGROUP is also set, of course.
>>
>> This won't make much of a difference, when the symbols are already in
>> the .config. So let's please not. It may be a 'y' for you by default,
>> but for lots of others it is not. Don't impose it on folks.
>
> This isn't about folkz, but HW. :-)

Is it everybody? No, it's a subset. Everybody adding a new driver wants
to default to y/m, and it's almost always wrong.

> I was thinking that it makes sense for the similar reason to why kyber
> and deadline are being built as default. Or are there any particular
> other reasons to why we build those in as default, but not BFQ?

deadline arguably makes sense as it's simple, and we should have one
default scheduler. kyber probably does not need to be default y. But
at least that one doesn't pull in other dependencies.

--
Jens Axboe



2023-01-30 10:42:36

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH] block: Default to build the BFQ I/O scheduler

On Fri, Jan 27, 2023 at 4:48 PM Jens Axboe <[email protected]> wrote:
> On 1/27/23 8:43 AM, Ulf Hansson wrote:
> > Today BFQ is widely used and it's also the default choice for some of the
> > single-queue-based storage devices. Therefore, let's make it more
> > convenient to build it as default, along with the other I/O schedulers.
> >
> > Let's also build the cgroup support for BFQ as default, as it's likely that
> > it's wanted too, assuming CONFIG_BLK_CGROUP is also set, of course.
>
> This won't make much of a difference, when the symbols are already in
> the .config. So let's please not. It may be a 'y' for you by default,
> but for lots of others it is not. Don't impose it on folks.

This part:

config BFQ_GROUP_IOSCHED
bool "BFQ hierarchical scheduling support"
depends on IOSCHED_BFQ && BLK_CGROUP
+ default y
select BLK_CGROUP_RWSTAT
help

should certainly be fine. If you select BFQ then it is helpful to get the
group scheduler as well if you have BLK_CGROUP, so Ulf could you
please send this part separately with that motivation?

For the selection of BFQ_IOSCHED:

As major distributions have it in their .configs it is kind of established
standard for them.

The general policy is that the kernel should
provide "sensible defaults", so it is easy to make correct choices even
if starting from scratch, relying on a few different out-of-kernel .configs
from different distros isn't helpful for someone starting from scratch.

What about default enabling it when enabling the relevant subsystems?

The udev script looks like so:

ACTION=="add", SUBSYSTEM=="block", ENV{DEVTYPE}=="disk", \
KERNEL=="mmcblk*[0-9]|msblk*[0-9]|mspblk*[0-9]|sd*[!0-9]|sr*", \
ATTR{queue/scheduler}="bfq"

i.e. drivers/[mmc|memstick|scsi|ata|cdrom]

This can be achieved by adding weak reverse dependencies to these
subsystems, meaning it will not be default-selected unless you make
use of one of them, and even then it can be manually removed (in
difference from using "select").

Example:

diff --git a/drivers/mmc/core/Kconfig b/drivers/mmc/core/Kconfig
index 6f25c34e4fec..52fe9d7c21cc 100644
--- a/drivers/mmc/core/Kconfig
+++ b/drivers/mmc/core/Kconfig
@@ -37,6 +37,7 @@ config PWRSEQ_SIMPLE
config MMC_BLOCK
tristate "MMC block device driver"
depends on BLOCK
+ imply IOSCHED_BFQ
default y
help
Say Y here to enable the MMC block device driver support.

Since all MMC devices will use BFQ on all major distributions this
makes sense to me. Also it is a policy that can be chosen by each
subsystem maintainer individually.

There is however the bigger problem of how to make a system come
up on eg MMC with BFQ on from the start, which I battled before but
it can be treated as an orthogonal problem.

Yours,
Linus Walleij

2023-01-30 10:45:09

by Linus Walleij

[permalink] [raw]
Subject: Re: [PATCH] block: Default to build the BFQ I/O scheduler

On Fri, Jan 27, 2023 at 9:10 PM Jens Axboe <[email protected]> wrote:

> On 1/27/23 8:58 AM, Ulf Hansson wrote:
> > On Fri, 27 Jan 2023 at 16:48, Jens Axboe <[email protected]> wrote:
> >> On 1/27/23 8:43 AM, Ulf Hansson wrote:
> >>> Today BFQ is widely used and it's also the default choice for some of the
> >>> single-queue-based storage devices. Therefore, let's make it more
> >>> convenient to build it as default, along with the other I/O schedulers.
> >>>
> >>> Let's also build the cgroup support for BFQ as default, as it's likely that
> >>> it's wanted too, assuming CONFIG_BLK_CGROUP is also set, of course.
> >>
> >> This won't make much of a difference, when the symbols are already in
> >> the .config. So let's please not. It may be a 'y' for you by default,
> >> but for lots of others it is not. Don't impose it on folks.
> >
> > This isn't about folkz, but HW. :-)
>
> Is it everybody? No, it's a subset. Everybody adding a new driver wants
> to default to y/m, and it's almost always wrong.

This isn't about individual drivers, as I showed from the udev rules
used by Fedora/Redhat it is clearly entire subsystems and hundreds
of drivers that desire this.

Ulf can certainly decide what is best for the MMC and memstick drivers.

Thus I think it is probably best to make each subsystem that desire
BFQ imply it.

Yours,
Linus Walleij

2023-01-30 11:57:27

by Ulf Hansson

[permalink] [raw]
Subject: Re: [PATCH] block: Default to build the BFQ I/O scheduler

On Fri, 27 Jan 2023 at 21:10, Jens Axboe <[email protected]> wrote:
>
> On 1/27/23 8:58 AM, Ulf Hansson wrote:
> > On Fri, 27 Jan 2023 at 16:48, Jens Axboe <[email protected]> wrote:
> >>
> >> On 1/27/23 8:43 AM, Ulf Hansson wrote:
> >>> Today BFQ is widely used and it's also the default choice for some of the
> >>> single-queue-based storage devices. Therefore, let's make it more
> >>> convenient to build it as default, along with the other I/O schedulers.
> >>>
> >>> Let's also build the cgroup support for BFQ as default, as it's likely that
> >>> it's wanted too, assuming CONFIG_BLK_CGROUP is also set, of course.
> >>
> >> This won't make much of a difference, when the symbols are already in
> >> the .config. So let's please not. It may be a 'y' for you by default,
> >> but for lots of others it is not. Don't impose it on folks.
> >
> > This isn't about folkz, but HW. :-)
>
> Is it everybody? No, it's a subset. Everybody adding a new driver wants
> to default to y/m, and it's almost always wrong.

BFQ and I/O schedulers aren't just simple "drivers'', but common
pieces of the storage stack.

As pointed out by Linus in his other reply, instead this strives
towards getting a sensible default configuration of the kernel.

>
> > I was thinking that it makes sense for the similar reason to why kyber
> > and deadline are being built as default. Or are there any particular
> > other reasons to why we build those in as default, but not BFQ?
>
> deadline arguably makes sense as it's simple, and we should have one
> default scheduler. kyber probably does not need to be default y. But
> at least that one doesn't pull in other dependencies.

Alright, so it sounds like it's rather a matter of the code's
complexity, whether it deserves to be default y or not. No?

I would rather let all the three I/O schedulers be default y, as it
looks like that would be the best default configuration of the kernel.
But it looks like we don't agree on that.

>
> --
> Jens Axboe
>
>

Kind regards
Uffe