Bug 196695 - elevator parameter allows mq io schedulers
Summary: elevator parameter allows mq io schedulers
Status: RESOLVED WILL_NOT_FIX
Alias: None
Product: IO/Storage
Classification: Unclassified
Component: Block Layer (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: Jens Axboe
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-08-17 23:53 UTC by loqs
Modified: 2017-10-31 20:06 UTC (History)
2 users (show)

See Also:
Kernel Version: 4.12 4.13
Tree: Mainline
Regression: No


Attachments
prevent mismatch of mq use for elevator and CONFIG_DEFAULT_IOSCHED (1.86 KB, patch)
2017-10-24 22:26 UTC, loqs
Details | Diff
minimal example fix (827 bytes, patch)
2017-10-24 22:31 UTC, loqs
Details | Diff
MQ vs non-mq load (2.27 KB, patch)
2017-10-24 23:22 UTC, Jens Axboe
Details | Diff
MQ vs non-mq load (3.27 KB, patch)
2017-10-24 23:26 UTC, Jens Axboe
Details | Diff

Description loqs 2017-08-17 23:53:03 UTC
Booting with the options scsi_mod.use_blk_mq=0 elevator=bfq (or elevator=mq-deadline or elevator=kyber) observe system fails to boot.
Alternately build a kernel with CONFIG_DEFAULT_IOSCHED="bfq" and boot with the option scsi_mod.use_blk_mq=0.
Second point is probably only relevant to custom kernels.
Documented on the arch linux forums here
https://bbs.archlinux.org/viewtopic.php?pid=1730573#p1730573
Reproduced on 4.13-rc5 and 4.12.7 with no additional patches.
Apologies if this is not a bug but intended behaviour.
Comment 1 Pablo Lezaeta Reyes 2017-08-18 04:12:28 UTC
I can confirm and also add that older versions of bfq don't need the scsi_mod.use_blk_mq to work and even used to work just exporting elevator=bfq so the issue could relly on how was implemented in the kernel.

I think kyber and bfq should be exposed indiferently from the scsi_mod.use_blk_mq option used since a system could need operate with any indirefent if one is optimized for other kind of system either for decire or mostly for testing prupose.
Comment 2 Jens Axboe 2017-10-18 16:30:35 UTC
The elevator= boot option is being phased out, we don't want to add it for the mq side of things.
Comment 3 loqs 2017-10-24 21:04:29 UTC
Could you add a check that mirrors d1a987f35ebf859a771ac530e95a89933b6fcce8
for the case where !e && !q->mq_ops && *chosen_elevator but chosen_elevator
is one of bfq,  mq_deadline or kyber until the elevator parameter is phased out please.
Related Line 236 seems to assumes e = elevator_get(CONFIG_DEFAULT_IOSCHED, false); will not return a mq elevator and unless the .Kconfig is adjusted that assumption seems valid not sure if the kernel should check e is not mq regardless.
Comment 4 Jens Axboe 2017-10-24 21:05:25 UTC
I'll be happy to take a patch that fixes up any issues that causes us not to boot.
Comment 5 loqs 2017-10-24 22:26:05 UTC
Created attachment 260375 [details]
prevent mismatch of mq use for elevator and CONFIG_DEFAULT_IOSCHED

This was the patch originally I drafted but it allows the elevator to be
mq as long as the queue is.
Comment 6 loqs 2017-10-24 22:31:32 UTC
Created attachment 260377 [details]
minimal example fix

This fix stops the mismatch with mq usage keeping the requirement that the elevator parameter and CONFIG_DEFAULT_IOSCHED must not be mq does not change
commenting or error messages but hopefully shows the intent of what I am trying to fix.
Comment 7 Jens Axboe 2017-10-24 23:18:04 UTC
That last patch, can you send that as a proper patch with commit message, etc? Looks fine to me, but you need to fix the whitespace issues (space after 'if').

If you want me to carry it to completion, that's fine too. Just let me know.
Comment 8 Jens Axboe 2017-10-24 23:22:48 UTC
Created attachment 260379 [details]
MQ vs non-mq load
Comment 9 Jens Axboe 2017-10-24 23:23:15 UTC
Created an alternative patch. Totally untested... But I prefer this approach, so we pass in whether we want MQ or not to the API. Can you give it a whirl?
Comment 10 Jens Axboe 2017-10-24 23:26:19 UTC
Created attachment 260381 [details]
MQ vs non-mq load

Lets push it a bit further...
Comment 11 loqs 2017-10-25 00:08:09 UTC
That patch fixes the issue.
When root device is using a none mq device and the elevator is an mq one eg bfq it now generates the error message "I/O scheduler bfq not found" and boots successfully
Comment 12 Jens Axboe 2017-10-25 00:40:12 UTC
Great, thanks for testing. I'll post it for review and include it upstream.
Comment 13 loqs 2017-10-31 19:58:48 UTC
If the patch is accepted into mainline 4.15 could it be backported to 4.14 stable?
Comment 14 Jens Axboe 2017-10-31 20:06:06 UTC
It could, but not sure it's grave enough to warrant stable backports.

Note You need to log in before you can comment on or make changes to this bug.