Skip to content

Commit adc0daa

Browse files
Mikulas Patockasnitm
Mikulas Patocka
authored andcommitted
dm: report suspended device during destroy
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: 48debaf ("dm: add writecache target") Cc: [email protected] # 4.18+ Reported-by: Corey Marthaler <[email protected]> Signed-off-by: Mikulas Patocka <[email protected]> Signed-off-by: Mike Snitzer <[email protected]>
1 parent 3918e06 commit adc0daa

File tree

3 files changed

+7
-8
lines changed

3 files changed

+7
-8
lines changed

drivers/md/dm-integrity.c

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -201,12 +201,13 @@ struct dm_integrity_c {
201201
__u8 log2_blocks_per_bitmap_bit;
202202

203203
unsigned char mode;
204-
int suspending;
205204

206205
int failed;
207206

208207
struct crypto_shash *internal_hash;
209208

209+
struct dm_target *ti;
210+
210211
/* these variables are locked with endio_wait.lock */
211212
struct rb_root in_progress;
212213
struct list_head wait_list;
@@ -2316,7 +2317,7 @@ static void integrity_writer(struct work_struct *w)
23162317
unsigned prev_free_sectors;
23172318

23182319
/* the following test is not needed, but it tests the replay code */
2319-
if (READ_ONCE(ic->suspending) && !ic->meta_dev)
2320+
if (unlikely(dm_suspended(ic->ti)) && !ic->meta_dev)
23202321
return;
23212322

23222323
spin_lock_irq(&ic->endio_wait.lock);
@@ -2377,7 +2378,7 @@ static void integrity_recalc(struct work_struct *w)
23772378

23782379
next_chunk:
23792380

2380-
if (unlikely(READ_ONCE(ic->suspending)))
2381+
if (unlikely(dm_suspended(ic->ti)))
23812382
goto unlock_ret;
23822383

23832384
range.logical_sector = le64_to_cpu(ic->sb->recalc_sector);
@@ -2805,8 +2806,6 @@ static void dm_integrity_postsuspend(struct dm_target *ti)
28052806

28062807
del_timer_sync(&ic->autocommit_timer);
28072808

2808-
WRITE_ONCE(ic->suspending, 1);
2809-
28102809
if (ic->recalc_wq)
28112810
drain_workqueue(ic->recalc_wq);
28122811

@@ -2835,8 +2834,6 @@ static void dm_integrity_postsuspend(struct dm_target *ti)
28352834
#endif
28362835
}
28372836

2838-
WRITE_ONCE(ic->suspending, 0);
2839-
28402837
BUG_ON(!RB_EMPTY_ROOT(&ic->in_progress));
28412838

28422839
ic->journal_uptodate = true;
@@ -3631,6 +3628,7 @@ static int dm_integrity_ctr(struct dm_target *ti, unsigned argc, char **argv)
36313628
}
36323629
ti->private = ic;
36333630
ti->per_io_data_size = sizeof(struct dm_integrity_io);
3631+
ic->ti = ti;
36343632

36353633
ic->in_progress = RB_ROOT;
36363634
INIT_LIST_HEAD(&ic->wait_list);

drivers/md/dm-writecache.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -842,7 +842,7 @@ static void writecache_suspend(struct dm_target *ti)
842842
}
843843
wc_unlock(wc);
844844

845-
flush_workqueue(wc->writeback_wq);
845+
drain_workqueue(wc->writeback_wq);
846846

847847
wc_lock(wc);
848848
if (flush_on_suspend)

drivers/md/dm.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2368,6 +2368,7 @@ static void __dm_destroy(struct mapped_device *md, bool wait)
23682368
map = dm_get_live_table(md, &srcu_idx);
23692369
if (!dm_suspended_md(md)) {
23702370
dm_table_presuspend_targets(map);
2371+
set_bit(DMF_SUSPENDED, &md->flags);
23712372
dm_table_postsuspend_targets(map);
23722373
}
23732374
/* dm_put_live_table must be before msleep, otherwise deadlock is possible */

0 commit comments

Comments
 (0)