Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1580347

Re: [PATCH BUGFIX] block: make elevator_get robust against cross blk/blk-mq choice

Show all headers | View raw

On 02/13/2017 11:28 PM, Jens Axboe wrote:
> On 02/13/2017 03:09 PM, Omar Sandoval wrote:
>> On Mon, Feb 13, 2017 at 10:01:07PM +0100, Paolo Valente wrote:
>>> If, at boot, a legacy I/O scheduler is chosen for a device using blk-mq,
>>> or, viceversa, a blk-mq scheduler is chosen for a device using blk, then
>>> that scheduler is set and initialized without any check, driving the
>>> system into an inconsistent state. This commit addresses this issue by
>>> letting elevator_get fail for these wrong cross choices.
>>>
>>> Signed-off-by: Paolo Valente <paolo.valente@linaro.org>
>>> ---
>>>  block/elevator.c | 26 ++++++++++++++++++--------
>>>  1 file changed, 18 insertions(+), 8 deletions(-)
>>
>> Hey, Paolo,
>>
>> How exactly are you triggering this? In __elevator_change(), we do check
>> for mq or not mq:
>>
>> 	if (!e->uses_mq && q->mq_ops) {
>> 		elevator_put(e);
>> 		return -EINVAL;
>> 	}
>> 	if (e->uses_mq && !q->mq_ops) {
>> 		elevator_put(e);
>> 		return -EINVAL;
>> 	}
>>
>> We don't ever appear to call elevator_init() with a specific scheduler
>> name, and for the default we switch off of q->mq_ops and use the
>> defaults from Kconfig:
>>
>> 	if (q->mq_ops && q->nr_hw_queues == 1)
>> 		e = elevator_get(CONFIG_DEFAULT_SQ_IOSCHED, false);
>> 	else if (q->mq_ops)
>> 		e = elevator_get(CONFIG_DEFAULT_MQ_IOSCHED, false);
>> 	else
>> 		e = elevator_get(CONFIG_DEFAULT_IOSCHED, false);
>>
>> 	if (!e) {
>> 		printk(KERN_ERR
>> 			"Default I/O scheduler not found. " \
>> 			"Using noop/none.\n");
>> 		e = elevator_get("noop", false);
>> 	}
>>
>> So I guess this could happen if someone manually changed those Kconfig
>> options, but I don't see what other case would make this happen, could
>> you please explain?
> 
> Was wondering the same - is it using the 'elevator=' boot parameter?
> Didn't look at that path just now, but that's the only one I could
> think of. If it is, I'd much prefer only using 'chosen_elevator' for
> the non-mq stuff, and the fix should be just that instead.
> 
[ .. ]
While we're at the topic:

Can't we use the same names for legacy and mq scheduler?
It's quite an unnecessary complication to have
'noop', 'deadline', and 'cfq' for legacy, but 'none' and 'mq-deadline'
for mq. If we could use 'noop' and 'deadline' for mq, too, the existing
settings or udev rules will continue to work and we wouldn't get any
annoying and pointless warnings here...

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH BUGFIX] attempt to fix wrong scheduler selection Paolo Valente <paolo.valente@linaro.org> - 2017-02-13 22:10 +0100
  [PATCH BUGFIX] block: make elevator_get robust against cross blk/blk-mq choice Paolo Valente <paolo.valente@linaro.org> - 2017-02-13 22:10 +0100
    Re: [PATCH BUGFIX] block: make elevator_get robust against cross  blk/blk-mq choice Bart Van Assche <Bart.VanAssche@sandisk.com> - 2017-02-13 22:20 +0100
    Re: [PATCH BUGFIX] block: make elevator_get robust against cross  blk/blk-mq choice Omar Sandoval <osandov@osandov.com> - 2017-02-13 23:10 +0100
      Re: [PATCH BUGFIX] block: make elevator_get robust against cross  blk/blk-mq choice Jens Axboe <axboe@kernel.dk> - 2017-02-13 23:30 +0100
        Re: [PATCH BUGFIX] block: make elevator_get robust against cross  blk/blk-mq choice Jens Axboe <axboe@kernel.dk> - 2017-02-14 00:20 +0100
          Re: [PATCH BUGFIX] block: make elevator_get robust against cross blk/blk-mq choice Paolo Valente <paolo.valente@linaro.org> - 2017-02-14 09:20 +0100
            Re: [PATCH BUGFIX] block: make elevator_get robust against cross  blk/blk-mq choice Jens Axboe <axboe@kernel.dk> - 2017-02-14 16:20 +0100
              Re: [PATCH BUGFIX] block: make elevator_get robust against cross blk/blk-mq choice Paolo Valente <paolo.valente@linaro.org> - 2017-02-14 16:50 +0100
        Re: [PATCH BUGFIX] block: make elevator_get robust against cross  blk/blk-mq choice Hannes Reinecke <hare@suse.de> - 2017-02-14 08:00 +0100
          Re: [PATCH BUGFIX] block: make elevator_get robust against cross  blk/blk-mq choice Omar Sandoval <osandov@osandov.com> - 2017-02-14 08:10 +0100
            Re: [PATCH BUGFIX] block: make elevator_get robust against cross  blk/blk-mq choice Hannes Reinecke <hare@suse.de> - 2017-02-14 08:20 +0100
          Re: [PATCH BUGFIX] block: make elevator_get robust against cross  blk/blk-mq choice Jens Axboe <axboe@kernel.dk> - 2017-02-14 16:20 +0100

csiph-web