[PATCH] mmc: core: Imply IOSCHED_BFQ

[PATCH] mmc: core: Imply IOSCHED_BFQ

[Linus Walleij]

If we enable the MMC/SD block layer, use Kconfig to imply the BFQ
I/O scheduler.

As all MMC/SD devices are single-queue, this is the scheduler that
users want so let's be helpful and make sure it gets
default-selected into a manual kernel configuration. It will still
need to be enabled at runtime (usually with udev scripts).

Cc: linux-block@vger.kernel.org
Cc: Paolo Valente <paolo.valente@linaro.org>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/mmc/core/Kconfig | 1 +
 1 file changed, 1 insertion(+)

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.
-- 
2.34.1

Re: [PATCH] mmc: core: Imply IOSCHED_BFQ

[Christoph Hellwig]

On Tue, Jan 31, 2023 at 09:47:42AM +0100, Linus Walleij wrote:
> If we enable the MMC/SD block layer, use Kconfig to imply the BFQ
> I/O scheduler.
> 
> As all MMC/SD devices are single-queue, this is the scheduler that
> users want so let's be helpful and make sure it gets
> default-selected into a manual kernel configuration. It will still
> need to be enabled at runtime (usually with udev scripts).

NAK.  Nothing requires a specific scheduler here - the scheduler is a
pure user choice.

Re: [PATCH] mmc: core: Imply IOSCHED_BFQ

[Linus Walleij]

On Tue, Jan 31, 2023 at 10:01 AM Christoph Hellwig <hch@infradead.org> wrote:
> On Tue, Jan 31, 2023 at 09:47:42AM +0100, Linus Walleij wrote:
> > If we enable the MMC/SD block layer, use Kconfig to imply the BFQ
> > I/O scheduler.
> >
> > As all MMC/SD devices are single-queue, this is the scheduler that
> > users want so let's be helpful and make sure it gets
> > default-selected into a manual kernel configuration. It will still
> > need to be enabled at runtime (usually with udev scripts).
>
> NAK.  Nothing requires a specific scheduler here - the scheduler is a
> pure user choice.

If MMC required a specific scheduler I would use

select IOSCHED_BFQ.

Now it doesn't require it, it will just perform better, so thus I use

imply IOSCHED_BFQ

the point with it is to help users make the right decisions, not
enforce them. fs/crypto/Kconfig does the same thing for example,
advice not enforce.

Yours,
Linus Walleij

Re: [PATCH] mmc: core: Imply IOSCHED_BFQ

[Christoph Hellwig]

On Tue, Jan 31, 2023 at 10:57:38AM +0100, Linus Walleij wrote:
> Now it doesn't require it, it will just perform better, so thus I use
> 
> imply IOSCHED_BFQ
> 
> the point with it is to help users make the right decisions, not
> enforce them. fs/crypto/Kconfig does the same thing for example,
> advice not enforce.

I'd really love to see how it always works better, as none of that
has actually been documented.  And given the wide variety of hardware
and workloads I also very much doubt it.

Re: [PATCH] mmc: core: Imply IOSCHED_BFQ

[Linus Walleij]

On Tue, Jan 31, 2023 at 2:20 PM Christoph Hellwig <hch@infradead.org> wrote:
> On Tue, Jan 31, 2023 at 10:57:38AM +0100, Linus Walleij wrote:
> > Now it doesn't require it, it will just perform better, so thus I use
> >
> > imply IOSCHED_BFQ
> >
> > the point with it is to help users make the right decisions, not
> > enforce them. fs/crypto/Kconfig does the same thing for example,
> > advice not enforce.
>
> I'd really love to see how it always works better, as none of that
> has actually been documented.

I guess I would need to turn that around and ask what
kind of documentation would convince you?

I guess this isn't enough:
https://www.kernel.org/doc/html/latest/block/bfq-iosched.html

To be clear, "works better" in this context means, solving a problem
for the interactive user, preventing random freezing of the UI on
resource-limited (memory, disk throughput) systems under high
I/O load.

Not maximizing throughput in general, which is the common
misunderstanding.

I personally ran into it, and blogged about it:
https://people.kernel.org/linusw/bfq-saved-me-from-thrashing

The phenomenon of freezing UI also happens on e.g. Android
while updating the apps in the background as illustrated by Paolo
here:
https://www.youtube.com/watch?v=ANfqNiJVoVE
More demos on more devices (mostly MMC/SD):
https://www.youtube.com/user/valentepaolo/videos

He also has several practical measurements of it and a test suite:
http://algogroup.unimore.it/people/paolo/disk_sched/results.php

There are also peer-reviewed IEEE articles and what not.

>  And given the wide variety of hardware
> and workloads I also very much doubt it.

The problem it solves appears on slow single-queue
devices where the scheduler cannot distinguish importance
between processes and treat all processes the same.

It does not try to solve the problem of maximizing throughput
on a real fast MQ device.

For MMC/SD-based systems this is very real, and that is why
this is implied by a patch to the MMC/SD subsystem.

Embedded interactive devices use MMC/SD-cards, while enterprise
storage systems does not.

This is why the udev rule in RedHat/Fedora looks like this:

60-block-scheduler.rules

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"

Targetting most slow single-queue devices with BFQ.

SuSE has something similar:
https://github.com/openSUSE/udev-extra-rules/blob/master/60-io-scheduler.rules

Yours,
Linus Walleij

Re: [PATCH] mmc: core: Imply IOSCHED_BFQ

[Christoph Hellwig]

On Tue, Jan 31, 2023 at 03:05:20PM +0100, Linus Walleij wrote:
> To be clear, "works better" in this context means, solving a problem
> for the interactive user, preventing random freezing of the UI on
> resource-limited (memory, disk throughput) systems under high
> I/O load.

Which already has nothing to do with the mmc driver.  And the rest
of your mail makes this even more clear.  You want bfq for interactive
systems with little resources and really shitty storage device, which
just happen to use mmc in your use case.

My use case for sd cards OTOH is extremely resource constrained systems
where I absolutely do not want a bloated I/O scheduler.  In fact I'd
love to be able to even compile the infrastructure for them away.

In other words:  you want distro policy to use bfq for your use case,
but that has no business being in the Kconfig.

Re: [PATCH] mmc: core: Imply IOSCHED_BFQ

[Linus Walleij]

On Tue, Jan 31, 2023 at 4:11 PM Christoph Hellwig <hch@infradead.org> wrote:
> On Tue, Jan 31, 2023 at 03:05:20PM +0100, Linus Walleij wrote:
> > To be clear, "works better" in this context means, solving a problem
> > for the interactive user, preventing random freezing of the UI on
> > resource-limited (memory, disk throughput) systems under high
> > I/O load.
>
> Which already has nothing to do with the mmc driver.  And the rest
> of your mail makes this even more clear.  You want bfq for interactive
> systems with little resources and really shitty storage device, which
> just happen to use mmc in your use case.

First time it helped me it was rotating rust actually, but yeah MMC/SD
is one of those.

> My use case for sd cards OTOH is extremely resource constrained systems
> where I absolutely do not want a bloated I/O scheduler.  In fact I'd
> love to be able to even compile the infrastructure for them away.

Hm I think you're mixing up different resource constraints here (that
your storage is slow does not mean your CPU is weak or that you have
little RAM) but I see your point.

What I think a lot of the debate is about is "abundance of resources"
systems vs "constrained resources" systems. Some are hard to keep
busy (such as MQ devices) other are hard to get access through
because of constant overload (such as MMC/SD-cards).

> In other words:  you want distro policy to use bfq for your use case,
> but that has no business being in the Kconfig.

Well *using* it is still the matter of a udev rule for an ordinary distro
as we have no mechanism to instruct the kernel to use any specific
scheduler with some subsystem. (I think we should have some hint
for that, for slow single channel devices for example.)

The Kconfig change is mainly about making it available for use,
for systems with MMC/SD-card drivers.

Yours,
Linus Walleij

Re: [PATCH] mmc: core: Imply IOSCHED_BFQ

[Christoph Hellwig]

On Wed, Feb 01, 2023 at 09:18:59AM +0100, Linus Walleij wrote:
> The Kconfig change is mainly about making it available for use,
> for systems with MMC/SD-card drivers.

But that's really not the job of Kconfig.  If you want an I/O schedule
enable it.  I thas absolutely no technical connection to the specific
block driver used.

Re: [PATCH] mmc: core: Imply IOSCHED_BFQ

[Arnd Bergmann]

On Tue, Jan 31, 2023, at 09:47, Linus Walleij wrote:
> If we enable the MMC/SD block layer, use Kconfig to imply the BFQ
> I/O scheduler.
>
> As all MMC/SD devices are single-queue, this is the scheduler that
> users want so let's be helpful and make sure it gets
> default-selected into a manual kernel configuration. It will still
> need to be enabled at runtime (usually with udev scripts).
>
> Cc: linux-block@vger.kernel.org
> Cc: Paolo Valente <paolo.valente@linaro.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/mmc/core/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> 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

As with most other uses of 'imply', this one does not do what you
think it does. Enabling MMC_BLOCK in 'make menuconfig' or similar
won't actually turn on IOSCHED_BFQ implicitly. The only difference
this makes is that it gets enabled in a 'make defconfig' run when
IOSCHED_BFQ is not already configured.

Just put it into the defconfig files instead.

     Arnd

Re: [PATCH] mmc: core: Imply IOSCHED_BFQ

[Linus Walleij]

On Wed, Feb 1, 2023 at 2:19 PM Arnd Bergmann <arnd@arndb.de> wrote:

> > 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
>
> As with most other uses of 'imply', this one does not do what you
> think it does. Enabling MMC_BLOCK in 'make menuconfig' or similar
> won't actually turn on IOSCHED_BFQ implicitly. The only difference
> this makes is that it gets enabled in a 'make defconfig' run when
> IOSCHED_BFQ is not already configured.

Incidentally that is all I do in my life, so it works as expected for me...
but OK.

> Just put it into the defconfig files instead.

Given that ARMv7 and ARM64/aarch64 often has MMC/SD-cards
I suppose I will start sending patches for these.

I see that m68k, MIPS Loongson, sh, um and s390 (!!) has already
enabled it in their defconfigs.

Yours,
Linus Walleij

Re: [PATCH] mmc: core: Imply IOSCHED_BFQ

[Ulf Hansson]

On Tue, 31 Jan 2023 at 09:47, Linus Walleij <linus.walleij@linaro.org> wrote:
>
> If we enable the MMC/SD block layer, use Kconfig to imply the BFQ
> I/O scheduler.
>
> As all MMC/SD devices are single-queue, this is the scheduler that
> users want so let's be helpful and make sure it gets
> default-selected into a manual kernel configuration. It will still
> need to be enabled at runtime (usually with udev scripts).
>
> Cc: linux-block@vger.kernel.org
> Cc: Paolo Valente <paolo.valente@linaro.org>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>

I have taken the various arguments (for and against), but I think
$subject patch makes sense to me. In the end, this is about moving
towards a more sensible default kernel configuration and the "imply"
approach works fine for me.

More importantly, $subject patch doesn't really hurt anything, as it's
still perfectly fine to build MMC without I/O schedulers and BFQ, for
those configurations that need this.

That said, applied for next, thanks!

Kind regards
Uffe


> ---
>  drivers/mmc/core/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> 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.
> --
> 2.34.1
>

Re: [PATCH] mmc: core: Imply IOSCHED_BFQ

[Jens Axboe]

On 2/2/23 8:22 AM, Ulf Hansson wrote:
> On Tue, 31 Jan 2023 at 09:47, Linus Walleij <linus.walleij@linaro.org> wrote:
>>
>> If we enable the MMC/SD block layer, use Kconfig to imply the BFQ
>> I/O scheduler.
>>
>> As all MMC/SD devices are single-queue, this is the scheduler that
>> users want so let's be helpful and make sure it gets
>> default-selected into a manual kernel configuration. It will still
>> need to be enabled at runtime (usually with udev scripts).
>>
>> Cc: linux-block@vger.kernel.org
>> Cc: Paolo Valente <paolo.valente@linaro.org>
>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> 
> I have taken the various arguments (for and against), but I think
> $subject patch makes sense to me. In the end, this is about moving
> towards a more sensible default kernel configuration and the "imply"
> approach works fine for me.
> 
> More importantly, $subject patch doesn't really hurt anything, as it's
> still perfectly fine to build MMC without I/O schedulers and BFQ, for
> those configurations that need this.
> 
> That said, applied for next, thanks!

It doesn't make sense, for all the reasons that Christoph applied.
But you guys seemed to already have your mind made up and ignoring
valid feedback, so...

-- 
Jens Axboe

Re: [PATCH] mmc: core: Imply IOSCHED_BFQ

[Linus Walleij]

On Thu, Feb 2, 2023 at 7:04 PM Jens Axboe <axboe@kernel.dk> wrote:
> On 2/2/23 8:22 AM, Ulf Hansson wrote:
> > On Tue, 31 Jan 2023 at 09:47, Linus Walleij <linus.walleij@linaro.org> wrote:
> >>
> >> If we enable the MMC/SD block layer, use Kconfig to imply the BFQ
> >> I/O scheduler.
> >>
> >> As all MMC/SD devices are single-queue, this is the scheduler that
> >> users want so let's be helpful and make sure it gets
> >> default-selected into a manual kernel configuration. It will still
> >> need to be enabled at runtime (usually with udev scripts).
> >>
> >> Cc: linux-block@vger.kernel.org
> >> Cc: Paolo Valente <paolo.valente@linaro.org>
> >> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> >
> > I have taken the various arguments (for and against), but I think
> > $subject patch makes sense to me. In the end, this is about moving
> > towards a more sensible default kernel configuration and the "imply"
> > approach works fine for me.
> >
> > More importantly, $subject patch doesn't really hurt anything, as it's
> > still perfectly fine to build MMC without I/O schedulers and BFQ, for
> > those configurations that need this.
> >
> > That said, applied for next, thanks!
>
> It doesn't make sense, for all the reasons that Christoph applied.
> But you guys seemed to already have your mind made up and ignoring
> valid feedback, so...

The history leading us here as I see it:

In 2017 or if it was 2016 I think Paolo and Ulf started to look into
BFQ for improving the user experience with MMC on embedded
devices such as phones, tablets etc. I.e all systems using it.

As BFQ was not accepted for the old block layer it was adopted
for the MQ rewrite, as I understood a bit as "the new CFQ" for
slow single-queued devices.

Then, the MMC subsystem was consequently
rewritten to use MQ to be able to take advantage of BFQ.
It's why we pushed the conversion to MQ. To the point of creating
social conflicts I might add. Not everyone loved converting MMC
to MQ.

Those changes are direct causes and effects, one to one.

And now today all that work has made it possible for the MMC
subsystem to perform as we wanted it to.

Most MMC systems are interactive human operator-facing
devices where interactivity matters. Any other MMC storage
is secondary, uncommon and unimportant. It is called
MultiMedia Card for a reason.

So for MMC BFQ is a sensible default as an interactivity
boosting scheduler. The kernel should provide sensible
defaults.

I do not see why it is not a sensible default for systems with
MMC.

Yours,
Linus Walleij