| From 2f36baa4a8721f28acea8eff02510e5f84cdece8 Mon Sep 17 00:00:00 2001 |
| From: Mikulas Patocka <mpatocka@redhat.com> |
| Date: Mon, 24 Feb 2020 10:20:28 +0100 |
| Subject: [PATCH] dm: report suspended device during destroy |
| |
| commit adc0daad366b62ca1bce3e2958a40b0b71a8b8b3 upstream. |
| |
| The function dm_suspended returns true if the target is suspended. |
| However, when the target is being suspended during unload, it returns |
| false. |
| |
| An example where this is a problem: the test "!dm_suspended(wc->ti)" in |
| writecache_writeback is not sufficient, because dm_suspended returns |
| zero while writecache_suspend is in progress. As is, without an |
| enhanced dm_suspended, simply switching from flush_workqueue to |
| drain_workqueue still emits warnings: |
| workqueue writecache-writeback: drain_workqueue() isn't complete after 10 tries |
| workqueue writecache-writeback: drain_workqueue() isn't complete after 100 tries |
| workqueue writecache-writeback: drain_workqueue() isn't complete after 200 tries |
| workqueue writecache-writeback: drain_workqueue() isn't complete after 300 tries |
| workqueue writecache-writeback: drain_workqueue() isn't complete after 400 tries |
| |
| writecache_suspend calls flush_workqueue(wc->writeback_wq) - this function |
| flushes the current work. However, the workqueue may re-queue itself and |
| flush_workqueue doesn't wait for re-queued works to finish. Because of |
| this - the function writecache_writeback continues execution after the |
| device was suspended and then concurrently with writecache_dtr, causing |
| a crash in writecache_writeback. |
| |
| We must use drain_workqueue - that waits until the work and all re-queued |
| works finish. |
| |
| As a prereq for switching to drain_workqueue, this commit fixes |
| dm_suspended to return true after the presuspend hook and before the |
| postsuspend hook - just like during a normal suspend. It allows |
| simplifying the dm-integrity and dm-writecache targets so that they |
| don't have to maintain suspended flags on their own. |
| |
| With this change use of drain_workqueue() can be used effectively. This |
| change was tested with the lvm2 testsuite and cryptsetup testsuite and |
| the are no regressions. |
| |
| Fixes: 48debafe4f2f ("dm: add writecache target") |
| Cc: stable@vger.kernel.org # 4.18+ |
| Reported-by: Corey Marthaler <cmarthal@redhat.com> |
| Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> |
| Signed-off-by: Mike Snitzer <snitzer@redhat.com> |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c |
| index 5914ae5c4ba3..41d1c70b5b81 100644 |
| --- a/drivers/md/dm-integrity.c |
| +++ b/drivers/md/dm-integrity.c |
| @@ -199,12 +199,13 @@ struct dm_integrity_c { |
| __u8 log2_blocks_per_bitmap_bit; |
| |
| unsigned char mode; |
| - int suspending; |
| |
| int failed; |
| |
| struct crypto_shash *internal_hash; |
| |
| + struct dm_target *ti; |
| + |
| /* these variables are locked with endio_wait.lock */ |
| struct rb_root in_progress; |
| struct list_head wait_list; |
| @@ -2298,7 +2299,7 @@ static void integrity_writer(struct work_struct *w) |
| unsigned prev_free_sectors; |
| |
| /* the following test is not needed, but it tests the replay code */ |
| - if (READ_ONCE(ic->suspending) && !ic->meta_dev) |
| + if (unlikely(dm_suspended(ic->ti)) && !ic->meta_dev) |
| return; |
| |
| spin_lock_irq(&ic->endio_wait.lock); |
| @@ -2360,7 +2361,7 @@ static void integrity_recalc(struct work_struct *w) |
| |
| next_chunk: |
| |
| - if (unlikely(READ_ONCE(ic->suspending))) |
| + if (unlikely(dm_suspended(ic->ti))) |
| goto unlock_ret; |
| |
| range.logical_sector = le64_to_cpu(ic->sb->recalc_sector); |
| @@ -2788,8 +2789,6 @@ static void dm_integrity_postsuspend(struct dm_target *ti) |
| |
| del_timer_sync(&ic->autocommit_timer); |
| |
| - WRITE_ONCE(ic->suspending, 1); |
| - |
| if (ic->recalc_wq) |
| drain_workqueue(ic->recalc_wq); |
| |
| @@ -2818,8 +2817,6 @@ static void dm_integrity_postsuspend(struct dm_target *ti) |
| #endif |
| } |
| |
| - WRITE_ONCE(ic->suspending, 0); |
| - |
| BUG_ON(!RB_EMPTY_ROOT(&ic->in_progress)); |
| |
| ic->journal_uptodate = true; |
| @@ -3604,6 +3601,7 @@ static int dm_integrity_ctr(struct dm_target *ti, unsigned argc, char **argv) |
| } |
| ti->private = ic; |
| ti->per_io_data_size = sizeof(struct dm_integrity_io); |
| + ic->ti = ti; |
| |
| ic->in_progress = RB_ROOT; |
| INIT_LIST_HEAD(&ic->wait_list); |
| diff --git a/drivers/md/dm-writecache.c b/drivers/md/dm-writecache.c |
| index 90c3bf95b287..763848b6b259 100644 |
| --- a/drivers/md/dm-writecache.c |
| +++ b/drivers/md/dm-writecache.c |
| @@ -839,7 +839,7 @@ static void writecache_suspend(struct dm_target *ti) |
| } |
| wc_unlock(wc); |
| |
| - flush_workqueue(wc->writeback_wq); |
| + drain_workqueue(wc->writeback_wq); |
| |
| wc_lock(wc); |
| if (flush_on_suspend) |
| diff --git a/drivers/md/dm.c b/drivers/md/dm.c |
| index f9a62e7eae47..14ca75ae68b5 100644 |
| --- a/drivers/md/dm.c |
| +++ b/drivers/md/dm.c |
| @@ -2390,6 +2390,7 @@ static void __dm_destroy(struct mapped_device *md, bool wait) |
| map = dm_get_live_table(md, &srcu_idx); |
| if (!dm_suspended_md(md)) { |
| dm_table_presuspend_targets(map); |
| + set_bit(DMF_SUSPENDED, &md->flags); |
| dm_table_postsuspend_targets(map); |
| } |
| /* dm_put_live_table must be before msleep, otherwise deadlock is possible */ |
| -- |
| 2.7.4 |
| |