elevator: Fix a race in elevator switching

There's a race between elevator switching and normal io operation.
    Because the allocation of struct elevator_queue and struct elevator_data
    don't in a atomic operation.So there are have chance to use NULL
    ->elevator_data.
    For example:
        Thread A:                               Thread B
        blk_queu_bio                            elevator_switch
        spin_lock_irq(q->queue_block)           elevator_alloc
        elv_merge                               elevator_init_fn

    Because call elevator_alloc, it can't hold queue_lock and the
    ->elevator_data is NULL.So at the same time, threadA call elv_merge and
    nedd some info of elevator_data.So the crash happened.

    Move the elevator_alloc into func elevator_init_fn, it make the
    operations in a atomic operation.

    Using the follow method can easy reproduce this bug
    1:dd if=/dev/sdb of=/dev/null
    2:while true;do echo noop > scheduler;echo deadline > scheduler;done

    The test method also use this method.

Signed-off-by: Jianpeng Ma <majianpeng@gmail.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>

1 files changed, 5 insertions, 20 deletions

diff --git a/block/elevator.c b/block/elevator.c
index eba5b04c29b1..668394d18588 100644
--- a/block/elevator.c
+++ b/block/elevator.c
@@ -150,7 +150,7 @@ void __init load_default_elevator_module(void)
 
 static struct kobj_type elv_ktype;
 
-static struct elevator_queue *elevator_alloc(struct request_queue *q,
+struct elevator_queue *elevator_alloc(struct request_queue *q,
                                   struct elevator_type *e)
 {
         struct elevator_queue *eq;
@@ -170,6 +170,7 @@ err:
         elevator_put(e);
         return NULL;
 }
+EXPORT_SYMBOL(elevator_alloc);
 
 static void elevator_release(struct kobject *kobj)
 {
@@ -221,16 +222,7 @@ int elevator_init(struct request_queue *q, char *name)
                 }
         }
 
-        q->elevator = elevator_alloc(q, e);
-        if (!q->elevator)
-                return -ENOMEM;
-
-        err = e->ops.elevator_init_fn(q);
-        if (err) {
-                kobject_put(&q->elevator->kobj);
-                return err;
-        }
-
+        err = e->ops.elevator_init_fn(q, e);
         return 0;
 }
 EXPORT_SYMBOL(elevator_init);
@@ -935,16 +927,9 @@ static int elevator_switch(struct request_queue *q, struct elevator_type *new_e)
         spin_unlock_irq(q->queue_lock);
 
         /* allocate, init and register new elevator */
-        err = -ENOMEM;
-        q->elevator = elevator_alloc(q, new_e);
-        if (!q->elevator)
-                goto fail_init;
-
-        err = new_e->ops.elevator_init_fn(q);
-        if (err) {
-                kobject_put(&q->elevator->kobj);
+        err = new_e->ops.elevator_init_fn(q, new_e);
+        if (err)
                 goto fail_init;
-        }
 
         if (registered) {
                 err = elv_register_queue(q);