| From d50235b7bc3ee0a0427984d763ea7534149531b4 Mon Sep 17 00:00:00 2001 |
| From: Jianpeng Ma <majianpeng@gmail.com> |
| Date: Wed, 3 Jul 2013 13:25:24 +0200 |
| Subject: elevator: Fix a race in elevator switching |
| |
| From: Jianpeng Ma <majianpeng@gmail.com> |
| |
| commit d50235b7bc3ee0a0427984d763ea7534149531b4 upstream. |
| |
| 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> |
| Cc: Jonghwan Choi <jhbird.choi@samsung.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| block/cfq-iosched.c | 17 ++++++++++++++--- |
| block/deadline-iosched.c | 16 +++++++++++++--- |
| block/elevator.c | 25 +++++-------------------- |
| block/noop-iosched.c | 17 ++++++++++++++--- |
| include/linux/elevator.h | 6 +++++- |
| 5 files changed, 51 insertions(+), 30 deletions(-) |
| |
| --- a/block/cfq-iosched.c |
| +++ b/block/cfq-iosched.c |
| @@ -4347,18 +4347,28 @@ static void cfq_exit_queue(struct elevat |
| kfree(cfqd); |
| } |
| |
| -static int cfq_init_queue(struct request_queue *q) |
| +static int cfq_init_queue(struct request_queue *q, struct elevator_type *e) |
| { |
| struct cfq_data *cfqd; |
| struct blkcg_gq *blkg __maybe_unused; |
| int i, ret; |
| + struct elevator_queue *eq; |
| + |
| + eq = elevator_alloc(q, e); |
| + if (!eq) |
| + return -ENOMEM; |
| |
| cfqd = kmalloc_node(sizeof(*cfqd), GFP_KERNEL | __GFP_ZERO, q->node); |
| - if (!cfqd) |
| + if (!cfqd) { |
| + kobject_put(&eq->kobj); |
| return -ENOMEM; |
| + } |
| + eq->elevator_data = cfqd; |
| |
| cfqd->queue = q; |
| - q->elevator->elevator_data = cfqd; |
| + spin_lock_irq(q->queue_lock); |
| + q->elevator = eq; |
| + spin_unlock_irq(q->queue_lock); |
| |
| /* Init root service tree */ |
| cfqd->grp_service_tree = CFQ_RB_ROOT; |
| @@ -4433,6 +4443,7 @@ static int cfq_init_queue(struct request |
| |
| out_free: |
| kfree(cfqd); |
| + kobject_put(&eq->kobj); |
| return ret; |
| } |
| |
| --- a/block/deadline-iosched.c |
| +++ b/block/deadline-iosched.c |
| @@ -337,13 +337,21 @@ static void deadline_exit_queue(struct e |
| /* |
| * initialize elevator private data (deadline_data). |
| */ |
| -static int deadline_init_queue(struct request_queue *q) |
| +static int deadline_init_queue(struct request_queue *q, struct elevator_type *e) |
| { |
| struct deadline_data *dd; |
| + struct elevator_queue *eq; |
| + |
| + eq = elevator_alloc(q, e); |
| + if (!eq) |
| + return -ENOMEM; |
| |
| dd = kmalloc_node(sizeof(*dd), GFP_KERNEL | __GFP_ZERO, q->node); |
| - if (!dd) |
| + if (!dd) { |
| + kobject_put(&eq->kobj); |
| return -ENOMEM; |
| + } |
| + eq->elevator_data = dd; |
| |
| INIT_LIST_HEAD(&dd->fifo_list[READ]); |
| INIT_LIST_HEAD(&dd->fifo_list[WRITE]); |
| @@ -355,7 +363,9 @@ static int deadline_init_queue(struct re |
| dd->front_merges = 1; |
| dd->fifo_batch = fifo_batch; |
| |
| - q->elevator->elevator_data = dd; |
| + spin_lock_irq(q->queue_lock); |
| + q->elevator = eq; |
| + spin_unlock_irq(q->queue_lock); |
| return 0; |
| } |
| |
| --- a/block/elevator.c |
| +++ b/block/elevator.c |
| @@ -150,7 +150,7 @@ void __init load_default_elevator_module |
| |
| 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->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,17 +927,10 @@ static int elevator_switch(struct reques |
| spin_unlock_irq(q->queue_lock); |
| |
| /* allocate, init and register new elevator */ |
| - err = -ENOMEM; |
| - q->elevator = elevator_alloc(q, new_e); |
| - if (!q->elevator) |
| + err = new_e->ops.elevator_init_fn(q, new_e); |
| + if (err) |
| goto fail_init; |
| |
| - err = new_e->ops.elevator_init_fn(q); |
| - if (err) { |
| - kobject_put(&q->elevator->kobj); |
| - goto fail_init; |
| - } |
| - |
| if (registered) { |
| err = elv_register_queue(q); |
| if (err) |
| --- a/block/noop-iosched.c |
| +++ b/block/noop-iosched.c |
| @@ -59,16 +59,27 @@ noop_latter_request(struct request_queue |
| return list_entry(rq->queuelist.next, struct request, queuelist); |
| } |
| |
| -static int noop_init_queue(struct request_queue *q) |
| +static int noop_init_queue(struct request_queue *q, struct elevator_type *e) |
| { |
| struct noop_data *nd; |
| + struct elevator_queue *eq; |
| + |
| + eq = elevator_alloc(q, e); |
| + if (!eq) |
| + return -ENOMEM; |
| |
| nd = kmalloc_node(sizeof(*nd), GFP_KERNEL, q->node); |
| - if (!nd) |
| + if (!nd) { |
| + kobject_put(&eq->kobj); |
| return -ENOMEM; |
| + } |
| + eq->elevator_data = nd; |
| |
| INIT_LIST_HEAD(&nd->queue); |
| - q->elevator->elevator_data = nd; |
| + |
| + spin_lock_irq(q->queue_lock); |
| + q->elevator = eq; |
| + spin_unlock_irq(q->queue_lock); |
| return 0; |
| } |
| |
| --- a/include/linux/elevator.h |
| +++ b/include/linux/elevator.h |
| @@ -7,6 +7,7 @@ |
| #ifdef CONFIG_BLOCK |
| |
| struct io_cq; |
| +struct elevator_type; |
| |
| typedef int (elevator_merge_fn) (struct request_queue *, struct request **, |
| struct bio *); |
| @@ -35,7 +36,8 @@ typedef void (elevator_put_req_fn) (stru |
| typedef void (elevator_activate_req_fn) (struct request_queue *, struct request *); |
| typedef void (elevator_deactivate_req_fn) (struct request_queue *, struct request *); |
| |
| -typedef int (elevator_init_fn) (struct request_queue *); |
| +typedef int (elevator_init_fn) (struct request_queue *, |
| + struct elevator_type *e); |
| typedef void (elevator_exit_fn) (struct elevator_queue *); |
| |
| struct elevator_ops |
| @@ -155,6 +157,8 @@ extern int elevator_init(struct request_ |
| extern void elevator_exit(struct elevator_queue *); |
| extern int elevator_change(struct request_queue *, const char *); |
| extern bool elv_rq_merge_ok(struct request *, struct bio *); |
| +extern struct elevator_queue *elevator_alloc(struct request_queue *, |
| + struct elevator_type *); |
| |
| /* |
| * Helper functions. |