| Message ID | 20220926220134.2633692-1-khazhy@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block: allow specifying default iosched in config | expand |
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
Gentle ping now that the merge window is open :)
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 ? > } > > /*
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,
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 >
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.
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 >>
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 >
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.
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
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!
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); } /*
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(-)