Commit 2673b4cf5d59c3ee5e0c12f6d734d38770324dc4
Committed by
Wu Fengguang
1 parent
62aa2b537c
Exists in
master
and in
20 other branches
backing-dev: fix wakeup timer races with bdi_unregister()
While 7a401a972df8e18 ("backing-dev: ensure wakeup_timer is deleted") addressed the problem of the bdi being freed with a queued wakeup timer, there are other races that could happen if the wakeup timer expires after/during bdi_unregister(), before bdi_destroy() is called. wakeup_timer_fn() could attempt to wakeup a task which has already has been freed, or could access a NULL bdi->dev via the wake_forker_thread tracepoint. Cc: <stable@kernel.org> Cc: Jens Axboe <axboe@kernel.dk> Reported-by: Chanho Min <chanho.min@lge.com> Reviewed-by: Namjae Jeon <linkinjeon@gmail.com> Signed-off-by: Rabin Vincent <rabin@rab.in> Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
Showing 1 changed file with 18 additions and 5 deletions Side-by-side Diff
mm/backing-dev.c
... | ... | @@ -318,7 +318,7 @@ |
318 | 318 | if (bdi->wb.task) { |
319 | 319 | trace_writeback_wake_thread(bdi); |
320 | 320 | wake_up_process(bdi->wb.task); |
321 | - } else { | |
321 | + } else if (bdi->dev) { | |
322 | 322 | /* |
323 | 323 | * When bdi tasks are inactive for long time, they are killed. |
324 | 324 | * In this case we have to wake-up the forker thread which |
... | ... | @@ -584,6 +584,8 @@ |
584 | 584 | */ |
585 | 585 | static void bdi_wb_shutdown(struct backing_dev_info *bdi) |
586 | 586 | { |
587 | + struct task_struct *task; | |
588 | + | |
587 | 589 | if (!bdi_cap_writeback_dirty(bdi)) |
588 | 590 | return; |
589 | 591 | |
... | ... | @@ -602,8 +604,13 @@ |
602 | 604 | * Finally, kill the kernel thread. We don't need to be RCU |
603 | 605 | * safe anymore, since the bdi is gone from visibility. |
604 | 606 | */ |
605 | - if (bdi->wb.task) | |
606 | - kthread_stop(bdi->wb.task); | |
607 | + spin_lock_bh(&bdi->wb_lock); | |
608 | + task = bdi->wb.task; | |
609 | + bdi->wb.task = NULL; | |
610 | + spin_unlock_bh(&bdi->wb_lock); | |
611 | + | |
612 | + if (task) | |
613 | + kthread_stop(task); | |
607 | 614 | } |
608 | 615 | |
609 | 616 | /* |
... | ... | @@ -623,7 +630,9 @@ |
623 | 630 | |
624 | 631 | void bdi_unregister(struct backing_dev_info *bdi) |
625 | 632 | { |
626 | - if (bdi->dev) { | |
633 | + struct device *dev = bdi->dev; | |
634 | + | |
635 | + if (dev) { | |
627 | 636 | bdi_set_min_ratio(bdi, 0); |
628 | 637 | trace_writeback_bdi_unregister(bdi); |
629 | 638 | bdi_prune_sb(bdi); |
630 | 639 | |
... | ... | @@ -632,8 +641,12 @@ |
632 | 641 | if (!bdi_cap_flush_forker(bdi)) |
633 | 642 | bdi_wb_shutdown(bdi); |
634 | 643 | bdi_debug_unregister(bdi); |
635 | - device_unregister(bdi->dev); | |
644 | + | |
645 | + spin_lock_bh(&bdi->wb_lock); | |
636 | 646 | bdi->dev = NULL; |
647 | + spin_unlock_bh(&bdi->wb_lock); | |
648 | + | |
649 | + device_unregister(dev); | |
637 | 650 | } |
638 | 651 | } |
639 | 652 | EXPORT_SYMBOL(bdi_unregister); |