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

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

[Ulf Hansson]

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 <ulf.hansson@linaro.org>
---
 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

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

[Jens Axboe]

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

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

[Ulf Hansson]

On Fri, 27 Jan 2023 at 16:48, Jens Axboe <axboe@kernel.dk> 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

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

[Jens Axboe]

On 1/27/23 8:58 AM, Ulf Hansson wrote:
> On Fri, 27 Jan 2023 at 16:48, Jens Axboe <axboe@kernel.dk> 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

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

[Linus Walleij]

On Fri, Jan 27, 2023 at 4:48 PM Jens Axboe <axboe@kernel.dk> 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

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

[Linus Walleij]

On Fri, Jan 27, 2023 at 9:10 PM Jens Axboe <axboe@kernel.dk> wrote:

> On 1/27/23 8:58 AM, Ulf Hansson wrote:
> > On Fri, 27 Jan 2023 at 16:48, Jens Axboe <axboe@kernel.dk> 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

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

[Ulf Hansson]

On Fri, 27 Jan 2023 at 21:10, Jens Axboe <axboe@kernel.dk> wrote:
>
> On 1/27/23 8:58 AM, Ulf Hansson wrote:
> > On Fri, 27 Jan 2023 at 16:48, Jens Axboe <axboe@kernel.dk> 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