On 17/12/1 上午2:48, Michael Lyle wrote:

Hi--

On 11/30/2017 04:00 AM, 彭良彦 wrote:

quoted

On 17/11/23 上午8:42, 彭良彦 wrote:

quoted

If the backing device hasn't been detached due to exceptional stop
like power outage, the BDEV_STATE in super block can't be reset,
it will fail to register this backing device in next time, but the
opened bdev will be hold, this makes the backing device unaccessable
because of FMODE_EXCL until reboot.

Signed-off-by: Liangyan Peng <redacted>

This doesn't seem correct.

What is the exact problem you're seeing?  Is it, if the cache device is
missing, the backing device "leaks"?  I think this is intentional-- it
is waiting there for an appropriate cache device to be attached that
lets it leave the STATE_DIRTY and run.  That is, either the backing
device or cache device can be registered first, and if the backing
device is registered first it is supposed to "wait" and not run until
the cache shows up.

Thanks Mike.

Here is the issue reproduce step.
1. create a block device sdb by iSCSI
2. echo /dev/sdb > /sys/fs/bcache/register and succeed to register //sdb is the block dev create by iSCSI
3. disconnect network and reboot  // without network, SET_BDEV_STATE(&dc->sb, BDEV_STATE_NONE) will fail
4. create a block device sdb by iSCSI
5. echo /dev/sdb > /sys/fs/bcache/register and fail to register since bdev state is not reset to BDEV_STATE_NONE
6. make-bcache -B /dev/sdb --writeback --wipe-bcache
the last step will fail in original version since bdev handle is not released during register and open() is failed in make-bcache, with this patch, it will succeed

quoted

quoted

---
 drivers/md/bcache/super.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/md/bcache/super.c b/drivers/md/bcache/super.c
index 66669c8f4161..0b95a594c12e 100644
--- a/drivers/md/bcache/super.c
+++ b/drivers/md/bcache/super.c

@@ -1174,9 +1174,7 @@ static void register_bdev(struct cache_sb *sb, struct page *sb_page,
 	list_for_each_entry(c, &bch_cache_sets, list)
 		bch_cached_dev_attach(dc, c);
 
-	if (BDEV_STATE(&dc->sb) == BDEV_STATE_NONE ||
-	    BDEV_STATE(&dc->sb) == BDEV_STATE_STALE)
-		bch_cached_dev_run(dc);
+	bch_cached_dev_run(dc);

You've moved the check out of here into the enclosing...

quoted

quoted

 
 	return;
 err:

@@ -1982,7 +1980,16 @@ static ssize_t register_bcache(struct kobject *k, struct kobj_attribute *attr,
 		goto err_close;
 
 	if (SB_IS_BDEV(sb)) {
-		struct cached_dev *dc = kzalloc(sizeof(*dc), GFP_KERNEL);
+		struct cached_dev *dc = NULL;
+
+		if (BDEV_STATE(sb) != BDEV_STATE_NONE &&
+			BDEV_STATE(sb) != BDEV_STATE_STALE) {
+			blkdev_put(bdev, FMODE_READ|FMODE_WRITE|FMODE_EXCL);
+			err = "invalid super block";
+			pr_info("Failed to register %s: %s", path, err);
+			goto out;
+		}

Here we check the state-- before bch_cached_dev_attach is called.  So if
the state is BDEV_STATE_DIRTY or BDEV_STATE_CLEAN there is no chance to
ever transition to _NONE/_STALE and run the cache.  This will make it
impossible to ever start up the cache after power failure.

Mike