diff mbox

elevator: allow specifying elevator by config boot param for blk-mq devices

Message ID 20170712185708.5987-1-yourbestfriend@openmailbox.org (mailing list archive)
State New, archived
Headers show

Commit Message

Alex Ivanov July 12, 2017, 6:57 p.m. UTC
It's now makes sense to use elevator boot argument when blk-mq is in use,
since there is now a bunch of schedulers for it (deadline, kyber, bfq, none).

Signed-off-by: Alex Ivanov <yourbestfriend@openmailbox.org>
---
 block/elevator.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Jens Axboe July 12, 2017, 10:51 p.m. UTC | #1
On 07/12/2017 12:57 PM, Alex Ivanov wrote:
> It's now makes sense to use elevator boot argument when blk-mq is in use,
> since there is now a bunch of schedulers for it (deadline, kyber, bfq, none).

No, that boot option was a mistake, let's not propagate that to mq
scheduling as well.
Johannes Thumshirn July 13, 2017, 10:11 a.m. UTC | #2
On Wed, Jul 12, 2017 at 04:51:33PM -0600, Jens Axboe wrote:
> No, that boot option was a mistake, let's not propagate that to mq
> scheduling as well.

Can you please explain why? I as well have requests from our users to
select the mq schedulers on the command line.

Thanks,
	Johannes
Jens Axboe July 13, 2017, 2:02 p.m. UTC | #3
On 07/13/2017 04:11 AM, Johannes Thumshirn wrote:
> On Wed, Jul 12, 2017 at 04:51:33PM -0600, Jens Axboe wrote:
>> No, that boot option was a mistake, let's not propagate that to mq
>> scheduling as well.
> 
> Can you please explain why? I as well have requests from our users to
> select the mq schedulers on the command line.

Because it sucks as an interface - there's no way to apply different
settings to different devices, and using the kernel command line to
control something like this is ugly. It never should have been done.

The sysfs interface, either manually, scripted, or through udev,
makes a lot more sense.
Paolo Valente July 14, 2017, 5:12 a.m. UTC | #4
> Il giorno 13 lug 2017, alle ore 16:02, Jens Axboe <axboe@kernel.dk> ha scritto:
> 
> On 07/13/2017 04:11 AM, Johannes Thumshirn wrote:
>> On Wed, Jul 12, 2017 at 04:51:33PM -0600, Jens Axboe wrote:
>>> No, that boot option was a mistake, let's not propagate that to mq
>>> scheduling as well.
>> 
>> Can you please explain why? I as well have requests from our users to
>> select the mq schedulers on the command line.
> 
> Because it sucks as an interface - there's no way to apply different
> settings to different devices, and using the kernel command line to
> control something like this is ugly. It never should have been done.
> 
> The sysfs interface, either manually, scripted, or through udev,
> makes a lot more sense.
> 

One doubt: with the new interface, and using, I guess, udev, is it
still possible to control which I/O scheduler is actually used during
all the boot process, or at least almost all of it?

Thanks,
Paolo

> -- 
> Jens Axboe
>
Johannes Thumshirn July 14, 2017, 6:40 a.m. UTC | #5
On Thu, Jul 13, 2017 at 08:02:41AM -0600, Jens Axboe wrote:
> Because it sucks as an interface - there's no way to apply different
> settings to different devices, and using the kernel command line to
> control something like this is ugly. It never should have been done.
> 
> The sysfs interface, either manually, scripted, or through udev,
> makes a lot more sense.

Not that I disagree with your reasons, but not being able to select a
mq scheduler confuses quite some users out there.

Byte,
	Johannes
Jens Axboe July 14, 2017, 2:26 p.m. UTC | #6
On 07/14/2017 12:40 AM, Johannes Thumshirn wrote:
> On Thu, Jul 13, 2017 at 08:02:41AM -0600, Jens Axboe wrote:
>> Because it sucks as an interface - there's no way to apply different
>> settings to different devices, and using the kernel command line to
>> control something like this is ugly. It never should have been done.
>>
>> The sysfs interface, either manually, scripted, or through udev,
>> makes a lot more sense.
> 
> Not that I disagree with your reasons, but not being able to select a
> mq scheduler confuses quite some users out there.

Maybe we can add something about the API being deprecated, if someone
tries to select an mq scheduler. The old scheduler will die in due
course once that path goes away, and once that happens, we can kill
the parameter.
Johannes Thumshirn July 17, 2017, 6:55 a.m. UTC | #7
On Fri, Jul 14, 2017 at 08:26:52AM -0600, Jens Axboe wrote:
 
> Maybe we can add something about the API being deprecated, if someone
> tries to select an mq scheduler. The old scheduler will die in due
> course once that path goes away, and once that happens, we can kill
> the parameter.

Yes this sounds like a good idea.
diff mbox

Patch

diff --git a/block/elevator.c b/block/elevator.c
index 4bb2f0c93fa6..ef6b27f13dfd 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -208,12 +208,12 @@  int elevator_init(struct request_queue *q, char *name)
 	}
 
 	/*
-	 * Use the default elevator specified by config boot param for
-	 * non-mq devices, or by config option. Don't try to load modules
+	 * Use the default elevator specified by config boot param
+	 * or by config option. Don't try to load modules
 	 * as we could be running off async and request_module() isn't
 	 * allowed from async.
 	 */
-	if (!e && !q->mq_ops && *chosen_elevator) {
+	if (!e && *chosen_elevator) {
 		e = elevator_get(chosen_elevator, false);
 		if (!e)
 			printk(KERN_ERR "I/O scheduler %s not found\n",