forked from mirrors/linux
		
	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: 48debafe4f ("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>
			
			
This commit is contained in:
		
							parent
							
								
									3918e0667b
								
							
						
					
					
						commit
						adc0daad36
					
				
					 3 changed files with 7 additions and 8 deletions
				
			
		| 
						 | 
				
			
			@ -201,12 +201,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;
 | 
			
		||||
| 
						 | 
				
			
			@ -2316,7 +2317,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);
 | 
			
		||||
| 
						 | 
				
			
			@ -2377,7 +2378,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);
 | 
			
		||||
| 
						 | 
				
			
			@ -2805,8 +2806,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);
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			@ -2835,8 +2834,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;
 | 
			
		||||
| 
						 | 
				
			
			@ -3631,6 +3628,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);
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -842,7 +842,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)
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -2368,6 +2368,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 */
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
		Reference in a new issue