diff mbox series

mmc: core: Imply IOSCHED_BFQ

Message ID 20230131084742.1038135-1-linus.walleij@linaro.org
State New
Headers show
Series mmc: core: Imply IOSCHED_BFQ | expand

Commit Message

Linus Walleij Jan. 31, 2023, 8:47 a.m. UTC
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(+)

Comments

'Christoph Hellwig' Jan. 31, 2023, 3:11 p.m. UTC | #1
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.
Linus Walleij Feb. 1, 2023, 8:18 a.m. UTC | #2
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
'Christoph Hellwig' Feb. 1, 2023, 1:08 p.m. UTC | #3
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.
Arnd Bergmann Feb. 1, 2023, 1:19 p.m. UTC | #4
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
Linus Walleij Feb. 1, 2023, 2:44 p.m. UTC | #5
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
Ulf Hansson Feb. 2, 2023, 3:22 p.m. UTC | #6
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
>
Jens Axboe Feb. 2, 2023, 6:04 p.m. UTC | #7
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...
Linus Walleij Feb. 2, 2023, 10:14 p.m. UTC | #8
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
diff mbox series

Patch

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.