diff options
author | Tejun Heo <tj@kernel.org> | 2015-07-01 20:53:37 -0400 |
---|---|---|
committer | Jens Axboe <axboe@fb.com> | 2015-07-02 10:46:00 -0400 |
commit | a20135ffbc44545596f9b99c970de097fb497bdd (patch) | |
tree | 18d7267ad1c0b0d603b4ca9cb1e43755c344f632 /mm/backing-dev.c | |
parent | a13f35e8714009145e32ebe2bf25b84e1376e314 (diff) |
writeback: don't drain bdi_writeback_congested on bdi destruction
52ebea749aae ("writeback: make backing_dev_info host cgroup-specific
bdi_writebacks") made bdi (backing_dev_info) host per-cgroup wb's
(bdi_writeback's). As the congested state needs to be per-wb and
referenced from blkcg side and multiple wbs, the patch made all
non-root cong's (bdi_writeback_congested's) reference counted and
indexed on bdi.
When a bdi is destroyed, cgwb_bdi_destroy() tries to drain all
non-root cong's; however, this can hang indefinitely because wb's can
also be referenced from blkcg_gq's which are destroyed after bdi
destruction is complete.
This patch fixes the bug by updating bdi destruction to not wait for
cong's to drain. A cong is unlinked from bdi->cgwb_congested_tree on
bdi destuction regardless of its reference count as the bdi may go
away any point after destruction. wb_congested_put() checks whether
the cong is already unlinked on release.
Signed-off-by: Tejun Heo <tj@kernel.org>
Reported-by: Jon Christopherson <jon@jons.org>
Link: https://bugzilla.kernel.org/show_bug.cgi?id=100681
Fixes: 52ebea749aae ("writeback: make backing_dev_info host cgroup-specific bdi_writebacks")
Tested-by: Jon Christopherson <jon@jons.org>
Signed-off-by: Jens Axboe <axboe@fb.com>
Diffstat (limited to 'mm/backing-dev.c')
-rw-r--r-- | mm/backing-dev.c | 22 |
1 files changed, 16 insertions, 6 deletions
diff --git a/mm/backing-dev.c b/mm/backing-dev.c index 51cc461e7256..dac5bf59309d 100644 --- a/mm/backing-dev.c +++ b/mm/backing-dev.c | |||
@@ -425,7 +425,6 @@ retry: | |||
425 | new_congested = NULL; | 425 | new_congested = NULL; |
426 | rb_link_node(&congested->rb_node, parent, node); | 426 | rb_link_node(&congested->rb_node, parent, node); |
427 | rb_insert_color(&congested->rb_node, &bdi->cgwb_congested_tree); | 427 | rb_insert_color(&congested->rb_node, &bdi->cgwb_congested_tree); |
428 | atomic_inc(&bdi->usage_cnt); | ||
429 | goto found; | 428 | goto found; |
430 | } | 429 | } |
431 | 430 | ||
@@ -456,7 +455,6 @@ found: | |||
456 | */ | 455 | */ |
457 | void wb_congested_put(struct bdi_writeback_congested *congested) | 456 | void wb_congested_put(struct bdi_writeback_congested *congested) |
458 | { | 457 | { |
459 | struct backing_dev_info *bdi = congested->bdi; | ||
460 | unsigned long flags; | 458 | unsigned long flags; |
461 | 459 | ||
462 | local_irq_save(flags); | 460 | local_irq_save(flags); |
@@ -465,12 +463,15 @@ void wb_congested_put(struct bdi_writeback_congested *congested) | |||
465 | return; | 463 | return; |
466 | } | 464 | } |
467 | 465 | ||
468 | rb_erase(&congested->rb_node, &congested->bdi->cgwb_congested_tree); | 466 | /* bdi might already have been destroyed leaving @congested unlinked */ |
467 | if (congested->bdi) { | ||
468 | rb_erase(&congested->rb_node, | ||
469 | &congested->bdi->cgwb_congested_tree); | ||
470 | congested->bdi = NULL; | ||
471 | } | ||
472 | |||
469 | spin_unlock_irqrestore(&cgwb_lock, flags); | 473 | spin_unlock_irqrestore(&cgwb_lock, flags); |
470 | kfree(congested); | 474 | kfree(congested); |
471 | |||
472 | if (atomic_dec_and_test(&bdi->usage_cnt)) | ||
473 | wake_up_all(&cgwb_release_wait); | ||
474 | } | 475 | } |
475 | 476 | ||
476 | static void cgwb_release_workfn(struct work_struct *work) | 477 | static void cgwb_release_workfn(struct work_struct *work) |
@@ -675,13 +676,22 @@ static int cgwb_bdi_init(struct backing_dev_info *bdi) | |||
675 | static void cgwb_bdi_destroy(struct backing_dev_info *bdi) | 676 | static void cgwb_bdi_destroy(struct backing_dev_info *bdi) |
676 | { | 677 | { |
677 | struct radix_tree_iter iter; | 678 | struct radix_tree_iter iter; |
679 | struct bdi_writeback_congested *congested, *congested_n; | ||
678 | void **slot; | 680 | void **slot; |
679 | 681 | ||
680 | WARN_ON(test_bit(WB_registered, &bdi->wb.state)); | 682 | WARN_ON(test_bit(WB_registered, &bdi->wb.state)); |
681 | 683 | ||
682 | spin_lock_irq(&cgwb_lock); | 684 | spin_lock_irq(&cgwb_lock); |
685 | |||
683 | radix_tree_for_each_slot(slot, &bdi->cgwb_tree, &iter, 0) | 686 | radix_tree_for_each_slot(slot, &bdi->cgwb_tree, &iter, 0) |
684 | cgwb_kill(*slot); | 687 | cgwb_kill(*slot); |
688 | |||
689 | rbtree_postorder_for_each_entry_safe(congested, congested_n, | ||
690 | &bdi->cgwb_congested_tree, rb_node) { | ||
691 | rb_erase(&congested->rb_node, &bdi->cgwb_congested_tree); | ||
692 | congested->bdi = NULL; /* mark @congested unlinked */ | ||
693 | } | ||
694 | |||
685 | spin_unlock_irq(&cgwb_lock); | 695 | spin_unlock_irq(&cgwb_lock); |
686 | 696 | ||
687 | /* | 697 | /* |