On 17/12/1 上午2:48, Michael Lyle wrote:
> Hi--
>
> On 11/30/2017 04:00 AM, 彭良彦 wrote:
>> On 17/11/23 上午8:42, 彭良彦 wrote:
>>> 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 <liangyan.ply@xxxxxxxxxxxxxxx>
>
> 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
>>> ---
>>> 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...
>
>>>
>>> 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��.n��������+%������w��{.n�����{����w��ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
