Backing device detaching will write back dirty data and update super block when it's finished, but won't set any flag to superblock after detaching starts. If system shuts down before detaching finishs, later cache set would never know detaching ever started before; what's worse, if users think detaching is done after shutdown and remove SSD which still contains dirty data, user would have a difficult time to bring up backing device again.
This patch will block bcache_reboot, if any cached_dev is still detaching, to guarantee detaching is done before shutdown. Another way will be adding a flag to superblock, then next bootup will continue unfinished detaching, but this won't solve the above worse case scenario.
This patch has been verified ay my setup for several test cases. And below is the new screen printout during shutdown, in case writeback is still ongoing.
bcache: bcache_reboot() Stopping all devices:
bcache: bcache_reboot() Backing device detaching hasn't been finished yet, please wait.
bcache: cached_dev_detach_finish() Caching disabled for loop1
bcache: cache_set_free() Cache set 69f2e475-3afc-42bf-b94d-8beb40030f59 unregistered
bcache: bcache_reboot() Timeout waiting for devices to be closed
reboot: System halted
Signed-off-by: Jianjian Huo <samuel.huo@xxxxxxxxx>
---
drivers/md/bcache/super.c | 29 +++++++++++++++++++++++++++--
1 file changed, 27 insertions(+), 2 deletions(-)
diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index 5a78df8..92dcbfd 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c
@@ -1981,6 +1981,22 @@ err:
goto out;
}
+static bool should_block_reboot(void)
+{
+ struct cache_set *c, *tc;
+ struct cached_dev *dc, *t;
+
+ lockdep_assert_held(&bch_register_lock);
+
+ /*block reboot if any cached_dev is detaching*/
+ list_for_each_entry_safe(c, tc, &bch_cache_sets, list)
+ list_for_each_entry_safe(dc, t, &c->cached_devs, list)
+ if (test_bit(BCACHE_DEV_DETACHING, &dc->disk.flags))
+ return true;
+
+ return false;
+}
+
static int bcache_reboot(struct notifier_block *n, unsigned long code, void *x)
{
if (code == SYS_DOWN ||
@@ -2010,18 +2026,27 @@ static int bcache_reboot(struct notifier_block *n, unsigned long code, void *x)
/* What's a condition variable? */
while (1) {
long timeout = start + 2 * HZ - jiffies;
+ bool blocking = false;
stopped = list_empty(&bch_cache_sets) &&
list_empty(&uncached_devices);
- if (timeout < 0 || stopped)
+ blocking = should_block_reboot();
+
+ if ((timeout < 0 && !blocking) || stopped)
break;
prepare_to_wait(&unregister_wait, &wait,
TASK_UNINTERRUPTIBLE);
mutex_unlock(&bch_register_lock);
- schedule_timeout(timeout);
+ if (blocking) {
+ pr_notice("Backing device detaching hasn't "
+ "been finished yet, please wait.");
+ schedule();
+ } else {
+ schedule_timeout(timeout);
+ }
mutex_lock(&bch_register_lock);
}
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-bcache" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
