[PATCH] block: allow specifying default iosched in config

[PATCH] block: allow specifying default iosched in config

[Khazhismel Kumykov]

Setting IO scheduler at device init time in kernel is useful, and moving
this option into kernel config makes it possible to build different
kernels with different default schedulers from the same tree.

Order deadline->none->rest to retain current behavior of using "none" by
default if mq-deadline is not enabled.

Signed-off-by: Khazhismel Kumykov <khazhy@google.com>
---
checkpatch suggested more verbose help descriptions, but I felt it'd be
too much repeated from the main config options, so opted to leave them
out.

 block/Kconfig.iosched | 28 ++++++++++++++++++++++++++++
 block/elevator.c      |  2 +-
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/block/Kconfig.iosched b/block/Kconfig.iosched
index 615516146086..38a83282802a 100644
--- a/block/Kconfig.iosched
+++ b/block/Kconfig.iosched
@@ -43,4 +43,32 @@ config BFQ_CGROUP_DEBUG
 	Enable some debugging help. Currently it exports additional stat
 	files in a cgroup which can be useful for debugging.
 
+choice
+	prompt "Default I/O scheduler"
+	default DEFAULT_MQ_DEADLINE
+	help
+	  Select the I/O scheduler which will be used by default for block devices
+	  with a single hardware queue.
+
+config DEFAULT_MQ_DEADLINE
+	bool "MQ Deadline" if MQ_IOSCHED_DEADLINE=y
+
+config DEFAULT_NONE
+	bool "none"
+
+config DEFAULT_MQ_KYBER
+	bool "Kyber" if MQ_IOSCHED_KYBER=y
+
+config DEFAULT_BFQ
+	bool "BFQ" if IOSCHED_BFQ=y
+
+endchoice
+
+config MQ_DEFAULT_IOSCHED
+	string
+	default "mq-deadline" if DEFAULT_MQ_DEADLINE
+	default "none" if DEFAULT_NONE
+	default "kyber" if DEFAULT_MQ_KYBER
+	default "bfq" if DEFAULT_BFQ
+
 endmenu
diff --git a/block/elevator.c b/block/elevator.c
index c319765892bb..4137933dfd16 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -642,7 +642,7 @@ static struct elevator_type *elevator_get_default(struct request_queue *q)
 	    !blk_mq_is_shared_tags(q->tag_set->flags))
 		return NULL;
 
-	return elevator_get(q, "mq-deadline", false);
+	return elevator_get(q, CONFIG_MQ_DEFAULT_IOSCHED, false);
 }
 
 /*
-- 
2.37.3.998.g577e59143f-goog

Re: [PATCH] block: allow specifying default iosched in config

[Chaitanya Kulkarni]

On 9/26/22 15:01, Khazhismel Kumykov wrote:
> Setting IO scheduler at device init time in kernel is useful, and moving
> this option into kernel config makes it possible to build different
> kernels with different default schedulers from the same tree.
> 
> Order deadline->none->rest to retain current behavior of using "none" by
> default if mq-deadline is not enabled.
> 
> Signed-off-by: Khazhismel Kumykov <khazhy@google.com>
> ---

Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>

-ck

Re: [PATCH] block: allow specifying default iosched in config

[Khazhy Kumykov]

Gentle ping now that the merge window is open :)

Re: [PATCH] block: allow specifying default iosched in config

[Damien Le Moal]

On 2022/09/27 7:01, Khazhismel Kumykov wrote:
> Setting IO scheduler at device init time in kernel is useful, and moving
> this option into kernel config makes it possible to build different
> kernels with different default schedulers from the same tree.
> 
> Order deadline->none->rest to retain current behavior of using "none" by
> default if mq-deadline is not enabled.
> 
> Signed-off-by: Khazhismel Kumykov <khazhy@google.com>
> ---
> checkpatch suggested more verbose help descriptions, but I felt it'd be
> too much repeated from the main config options, so opted to leave them
> out.
> 
>  block/Kconfig.iosched | 28 ++++++++++++++++++++++++++++
>  block/elevator.c      |  2 +-
>  2 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/block/Kconfig.iosched b/block/Kconfig.iosched
> index 615516146086..38a83282802a 100644
> --- a/block/Kconfig.iosched
> +++ b/block/Kconfig.iosched
> @@ -43,4 +43,32 @@ config BFQ_CGROUP_DEBUG
>  	Enable some debugging help. Currently it exports additional stat
>  	files in a cgroup which can be useful for debugging.
>  
> +choice
> +	prompt "Default I/O scheduler"
> +	default DEFAULT_MQ_DEADLINE
> +	help
> +	  Select the I/O scheduler which will be used by default for block devices
> +	  with a single hardware queue.
> +
> +config DEFAULT_MQ_DEADLINE
> +	bool "MQ Deadline" if MQ_IOSCHED_DEADLINE=y
> +
> +config DEFAULT_NONE
> +	bool "none"
> +
> +config DEFAULT_MQ_KYBER
> +	bool "Kyber" if MQ_IOSCHED_KYBER=y
> +
> +config DEFAULT_BFQ
> +	bool "BFQ" if IOSCHED_BFQ=y
> +
> +endchoice
> +
> +config MQ_DEFAULT_IOSCHED
> +	string
> +	default "mq-deadline" if DEFAULT_MQ_DEADLINE
> +	default "none" if DEFAULT_NONE
> +	default "kyber" if DEFAULT_MQ_KYBER
> +	default "bfq" if DEFAULT_BFQ
> +
>  endmenu
> diff --git a/block/elevator.c b/block/elevator.c
> index c319765892bb..4137933dfd16 100644
> --- a/block/elevator.c
> +++ b/block/elevator.c
> @@ -642,7 +642,7 @@ static struct elevator_type *elevator_get_default(struct request_queue *q)
>  	    !blk_mq_is_shared_tags(q->tag_set->flags))
>  		return NULL;
>  
> -	return elevator_get(q, "mq-deadline", false);
> +	return elevator_get(q, CONFIG_MQ_DEFAULT_IOSCHED, false);

This will allow a configuration to specify bfq or kyber for all single queue
devices, which include SMR HDDs. Since these can only use mq-deadline (or none
if the user like living dangerously), this default config-based solution is not
OK in my opinion.

What is wrong with using a udev rule to set the default disk scheduler ? Most
distros do that already anyway, so this config may not even be that useful in
practice. What is the use case here ?

>  }
>  
>  /*

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH] block: allow specifying default iosched in config

[Jens Axboe]

On Mon, 26 Sep 2022 15:01:34 -0700, Khazhismel Kumykov wrote:
> Setting IO scheduler at device init time in kernel is useful, and moving
> this option into kernel config makes it possible to build different
> kernels with different default schedulers from the same tree.
> 
> Order deadline->none->rest to retain current behavior of using "none" by
> default if mq-deadline is not enabled.
> 
> [...]

Applied, thanks!

[1/1] block: allow specifying default iosched in config
      commit: ad9d3da2d07e0b2966e3ced843a0e2229410e26a

Best regards,
-- 
Jens Axboe

Re: [PATCH] block: allow specifying default iosched in config

[Khazhy Kumykov]

On Mon, Oct 3, 2022 at 11:12 PM Damien Le Moal
<damien.lemoal@opensource.wdc.com> wrote:
>
> This will allow a configuration to specify bfq or kyber for all single queue
> devices
That's the idea
> , which include SMR HDDs. Since these can only use mq-deadline (or none
> if the user like living dangerously), this default config-based solution is not
> OK in my opinion.
I don't think this is true - elevator_init_mq will call
elevator_get_by_features, not elevator_get_default for SMR hdds (and
other zoned devices), as it sets required_elevator_features.
>
> What is wrong with using a udev rule to set the default disk scheduler ? Most
> distros do that already anyway, so this config may not even be that useful in
> practice. What is the use case here ?
>
>
> --
> Damien Le Moal
> Western Digital Research
>

Re: [PATCH] block: allow specifying default iosched in config

[Jens Axboe]

On 10/4/22 10:42 AM, Jens Axboe wrote:
> On Mon, 26 Sep 2022 15:01:34 -0700, Khazhismel Kumykov wrote:
>> Setting IO scheduler at device init time in kernel is useful, and moving
>> this option into kernel config makes it possible to build different
>> kernels with different default schedulers from the same tree.
>>
>> Order deadline->none->rest to retain current behavior of using "none" by
>> default if mq-deadline is not enabled.
>>
>> [...]
> 
> Applied, thanks!
> 
> [1/1] block: allow specifying default iosched in config
>       commit: ad9d3da2d07e0b2966e3ced843a0e2229410e26a

I'm going to drop this one for now since we still have
active discussion on it and it's very late (actually too late)
to be debating 6.1.

-- 
Jens Axboe

Re: [PATCH] block: allow specifying default iosched in config

[Damien Le Moal]

On 10/5/22 03:33, Khazhy Kumykov wrote:
> On Mon, Oct 3, 2022 at 11:12 PM Damien Le Moal
> <damien.lemoal@opensource.wdc.com> wrote:
>>
>> This will allow a configuration to specify bfq or kyber for all single queue
>> devices
> That's the idea
>> , which include SMR HDDs. Since these can only use mq-deadline (or none
>> if the user like living dangerously), this default config-based solution is not
>> OK in my opinion.
> I don't think this is true - elevator_init_mq will call
> elevator_get_by_features, not elevator_get_default for SMR hdds (and
> other zoned devices), as it sets required_elevator_features.

OK, but my point remains: why not use a udev rule ? Why add a config
option to hardcode the default scheduler ? Most average users will have no
idea which one to choose...

>>
>> What is wrong with using a udev rule to set the default disk scheduler ? Most
>> distros do that already anyway, so this config may not even be that useful in
>> practice. What is the use case here ?
>>
>>
>> --
>> Damien Le Moal
>> Western Digital Research
>>

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH] block: allow specifying default iosched in config

[Khazhy Kumykov]

[-- Attachment #1: Type: text/plain, Size: 1716 bytes --]

On Tue, Oct 4, 2022 at 3:40 PM Damien Le Moal
<damien.lemoal@opensource.wdc.com> wrote:
>
> On 10/5/22 03:33, Khazhy Kumykov wrote:
> > On Mon, Oct 3, 2022 at 11:12 PM Damien Le Moal
> > <damien.lemoal@opensource.wdc.com> wrote:
> >>
> >> This will allow a configuration to specify bfq or kyber for all single queue
> >> devices
> > That's the idea
> >> , which include SMR HDDs. Since these can only use mq-deadline (or none
> >> if the user like living dangerously), this default config-based solution is not
> >> OK in my opinion.
> > I don't think this is true - elevator_init_mq will call
> > elevator_get_by_features, not elevator_get_default for SMR hdds (and
> > other zoned devices), as it sets required_elevator_features.
>
> OK, but my point remains: why not use a udev rule ? Why add a config
> option to hardcode the default scheduler ? Most average users will have no
> idea which one to choose...
The kernel already picks and hardcodes a default scheduler, my
thinking is: why not let folks choose? This was allowed in the old
block layer since 2005.

My first thought was it'd be handy to have the kernel just start up
with the scheduler we wanted, rather than rely on userspace to listen
to an event to fix it.
But, some userspaces do not run a udev daemon (e.g. linuxboot, to my
recollection, others), so would need to stand one up, just for this.

Khazhy
>
> >>
> >> What is wrong with using a udev rule to set the default disk scheduler ? Most
> >> distros do that already anyway, so this config may not even be that useful in
> >> practice. What is the use case here ?
> >>
> >>
> >> --
> >> Damien Le Moal
> >> Western Digital Research
> >>
>
> --
> Damien Le Moal
> Western Digital Research
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 3999 bytes --]

Re: [PATCH] block: allow specifying default iosched in config

[Christoph Hellwig]

On Tue, Oct 04, 2022 at 04:15:20PM -0700, Khazhy Kumykov wrote:
> The kernel already picks and hardcodes a default scheduler, my
> thinking is: why not let folks choose? This was allowed in the old
> block layer since 2005.

You can choose it using CONFIG_CMDLINE.  We can't add a config option
for every bloody default as that simply does not scale.

Re: [PATCH] block: allow specifying default iosched in config

[Khazhy Kumykov]

On Mon, Oct 10, 2022 at 12:20 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Tue, Oct 04, 2022 at 04:15:20PM -0700, Khazhy Kumykov wrote:
> > The kernel already picks and hardcodes a default scheduler, my
> > thinking is: why not let folks choose? This was allowed in the old
> > block layer since 2005.
>
> You can choose it using CONFIG_CMDLINE.  We can't add a config option
> for every bloody default as that simply does not scale.

Are you referring to elevator=? It looks like that needs to be
re-wired for blk-mq, but seems like a reasonable solution. I'll send a
new patch out.

Thanks,
Khazhy

Re: [PATCH] block: allow specifying default iosched in config

[Christoph Hellwig]

On Mon, Oct 17, 2022 at 10:22:59AM -0700, Khazhy Kumykov wrote:
> > > The kernel already picks and hardcodes a default scheduler, my
> > > thinking is: why not let folks choose? This was allowed in the old
> > > block layer since 2005.
> >
> > You can choose it using CONFIG_CMDLINE.  We can't add a config option
> > for every bloody default as that simply does not scale.
> 
> Are you referring to elevator=? It looks like that needs to be
> re-wired for blk-mq, but seems like a reasonable solution. I'll send a
> new patch out.

Yes, thanks!