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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <fff022da-72f2-0fdb-e792-8d75069441cc@opensource.wdc.com>
Date:   Tue, 4 Oct 2022 15:12:08 +0900
From:   Damien Le Moal <damien.lemoal@...nsource.wdc.com>
To:     Khazhismel Kumykov <khazhy@...omium.org>,
        Jens Axboe <axboe@...nel.dk>
Cc:     linux-block@...r.kernel.org, linux-kernel@...r.kernel.org,
        Khazhismel Kumykov <khazhy@...gle.com>
Subject: Re: [PATCH] block: allow specifying default iosched in config

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@...gle.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