Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
IronPort-SDR: +GCr15y/hJCDxofZk2pfk4FMWGM1ytWENpAXrMAKlwk6KcvHBu8Rv/lh96eS5+Y9pvLuYjD28b
 h5BIRkAAFpssdJb48X3zDmApo6542lHkrEseBglwGTZAEoy0ByH6uWAoK93QFPeJqO3VmanYmY
 kYMV34sTLkugLx9Z8AvI8W2va+m0h2yS+XEGP0mhCY2CnKHlz+f09xkDj5lYGLgmQ6soiX177H
 Hh/bHGo06Q5xpPpePiCMIPQdL8d57yBfxXrXZrPAtPozFDvH+XxmvztOHZfdiTqAMJ2D4LphpB
 UmdmPhMwaQsirfCBN4FIlVGG
IronPort-SDR: Qkri+TwlRAeXCYI4V+hdY+1avXsHYXepEPb+mWGpokjvL1/ZtSrLebhFamofU1WJPXASdgiyc/
 5lXe9AqkWsD4geJDzlWZaocW2/9q1Ny++UQvARIYpCV7WUwmzMJVxwQtEWsK8HVZ3LFT6dlE/7
 AUqIQiGXUpB29HsjRVsiwTWbgaRORmCiYjvkYp4gaITpKxu95ttlshKku6cm9yQ1yorWTTwyyT
 aRff6Z+xxi0gWc8V8RN/RgL2Bg4HNKVtN8NAZYyaHUJJDm0kvBxgvrsHOTbTUsbvIHTasXmsNz
 BIs=
WDCIronportException: Internal
Message-ID: <fff022da-72f2-0fdb-e792-8d75069441cc@opensource.wdc.com>
Date:   Tue, 4 Oct 2022 15:12:08 +0900
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.3.1
Subject: Re: [PATCH] block: allow specifying default iosched in config
Content-Language: en-US
To:     Khazhismel Kumykov <khazhy@chromium.org>,
        Jens Axboe <axboe@kernel.dk>
Cc:     linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
        Khazhismel Kumykov <khazhy@google.com>
References: <20220926220134.2633692-1-khazhy@google.com>
From:   Damien Le Moal <damien.lemoal@opensource.wdc.com>
Organization: Western Digital Research
In-Reply-To: <20220926220134.2633692-1-khazhy@google.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

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