MIME-Version: 1.0
References: <20230127154339.157460-1-ulf.hansson@linaro.org> <5743fb1b-eb4b-d169-a467-ee618bcd75f5@kernel.dk>
In-Reply-To: <5743fb1b-eb4b-d169-a467-ee618bcd75f5@kernel.dk>
From:   Linus Walleij <linus.walleij@linaro.org>
Date:   Mon, 30 Jan 2023 11:42:20 +0100
Message-ID: <CACRpkdbyOcTBbv+EmDcnMXDjgiP1HKpuYgMF_JhTMOnLj7MA3g@mail.gmail.com>
Subject: Re: [PATCH] block: Default to build the BFQ I/O scheduler
To:     Jens Axboe <axboe@kernel.dk>
Cc:     Ulf Hansson <ulf.hansson@linaro.org>, linux-block@vger.kernel.org,
        Paolo Valente <paolo.valente@linaro.org>,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Fri, Jan 27, 2023 at 4:48 PM Jens Axboe <axboe@kernel.dk> wrote:
> On 1/27/23 8:43=E2=80=AFAM, Ulf Hansson wrote:
> > Today BFQ is widely used and it's also the default choice for some of t=
he
> > 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=3D=3D"add", SUBSYSTEM=3D=3D"block", ENV{DEVTYPE}=3D=3D"disk", \
  KERNEL=3D=3D"mmcblk*[0-9]|msblk*[0-9]|mspblk*[0-9]|sd*[!0-9]|sr*", \
  ATTR{queue/scheduler}=3D"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