On 2017年12月08日 19:48, Anand Jain wrote: > > > On 12/08/2017 07:01 PM, Qu Wenruo wrote: >> >> >> On 2017年12月08日 18:39, Anand Jain wrote: >>> >>> >>> On 12/08/2017 04:17 PM, Qu Wenruo wrote: >>>> >>>> >>>> On 2017年12月08日 15:57, Anand Jain wrote: >>>>> -EXPERIMENTAL- >>>>> As of now when primary SB fails we won't self heal and would fail >>>>> mount, >>>>> this is an experimental patch which thinks why not go and read backup >>>>> copy. >>>> >>>> Just curious about in which real world case that backup super block can >>>> help. >>>> At least from what I see in mail list, only few cases where backup >>>> super >>>> helps. >>> >>> Theoretical design helps. I ended up in this situation though. And >>> ext4 has -o sb flag to manage this part. When we can expect EIO on >>> any part of the disk block why not on the LBA which contains primary >>> SB. And should we fail the mount for that reason ? No. >> >> And how do you ensure it's a btrfs? > > Hmm. You mean outside of btrfs ? I did experiment with wipe and then > using /etc/fstab to mount, and it did lead to btrfs, is that your > concern that it shouldn't have been. That looked surprising to me as > well, but then problem points at wipefs instead.
It's closer, but still doesn't reach the point.
It can be a mkfs, other than wipefs.
It's can be another case like:
One user used btrfs for a while
But bugs made him/her unhappy, and he/she turned to use xfs (whatever
the fs is) instead.
While he/she forgot to change its fstab, and rebooted the system.
And suddenly, it's btrfs again!
What a surprise.
>
>>
>>>
>>>> Despite that self super heal seems good, although I agree with
>>>> David, we
>>>> need a weaker but necessary check (magic and fsid from primary super?)
>>>> to ensure it's a valid btrfs before we use the backup supers.
>>>
>>> Of course, we already have btrfs_check_super_valid() to verify the SB,
>>> I don't understand why - how do we verify the SB should be the
>>> point of
>>> concern here, at all.
>>
>> The point here is, to distinguish an old and invalid btrfs (some other
>> valid fs mkfs beyond the old fs) from a valid btrfs with corrupted
>> primary fs.
>
> Ok. When you check all the SBs you would pick the one which has passed
> btrfs_check_super_valid() and has highest generation. Did I ans your
> concern ? If a SB does not pass btrfs_check_super_valid() its not a
> valid btrfs SB at all.
No, not really.
What if the first SB is a XFS one or even a fs you didn't ever hear?
Skip it and use the 2nd one?
This filesystem is not even btrfs anymore.
Thanks,
Qu
>
>
>> This the main concern here.
>> The difference between recovery and recognizing a bad btrfs is quite
>> important.
>
> btrfs_check_super_valid() is already doing that right ? The point here
> is, should we use the backup SB when btrfs_check_super_valid() fails on
> primary SB.
>
> Thanks, Anand
>
>> Thanks,
>> Qu
>>
>>>
>>> Thanks, Anand
>>>
>>>> Thanks,
>>>> Qu
>>>>
>>>>
>>>>>
>>>>> Signed-off-by: Anand Jain <anand.j...@oracle.com>
>>>>> ---
>>>>> fs/btrfs/disk-io.c | 8 +++++++-
>>>>> fs/btrfs/volumes.c | 10 +++++++---
>>>>> 2 files changed, 14 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>>>>> index 9b20c1f3563b..a791b8dfe8a8 100644
>>>>> --- a/fs/btrfs/disk-io.c
>>>>> +++ b/fs/btrfs/disk-io.c
>>>>> @@ -3190,7 +3190,7 @@ struct buffer_head *btrfs_read_dev_super(struct
>>>>> block_device *bdev)
>>>>> * So, we need to add a special mount option to scan for
>>>>> * later supers, using BTRFS_SUPER_MIRROR_MAX instead
>>>>> */
>>>>> - for (i = 0; i < 1; i++) {
>>>>> + for (i = 0; i < BTRFS_SUPER_MIRROR_MAX; i++) {
>>>>> ret = btrfs_read_dev_one_super(bdev, i, &bh);
>>>>> if (ret)
>>>>> continue;
>>>>> @@ -4015,11 +4015,17 @@ static int btrfs_check_super_valid(struct
>>>>> btrfs_fs_info *fs_info)
>>>>> ret = -EINVAL;
>>>>> }
>>>>> +#if 0
>>>>> + /*
>>>>> + * Need a way to check for any copy of SB, as its not a
>>>>> + * strong check, just ignore this for now.
>>>>> + */
>>>>> if (btrfs_super_bytenr(sb) != BTRFS_SUPER_INFO_OFFSET) {
>>>>> btrfs_err(fs_info, "super offset mismatch %llu != %u",
>>>>> btrfs_super_bytenr(sb), BTRFS_SUPER_INFO_OFFSET);
>>>>> ret = -EINVAL;
>>>>> }
>>>>> +#endif
>>>>> /*
>>>>> * Obvious sys_chunk_array corruptions, it must hold at least
>>>>> one key
>>>>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>>>>> index 9fa2539a8493..f368db94d62b 100644
>>>>> --- a/fs/btrfs/volumes.c
>>>>> +++ b/fs/btrfs/volumes.c
>>>>> @@ -1369,7 +1369,7 @@ int btrfs_scan_one_device(const char *path,
>>>>> fmode_t flags, void *holder,
>>>>> {
>>>>> struct btrfs_super_block *disk_super;
>>>>> struct block_device *bdev;
>>>>> - struct page *page;
>>>>> + struct buffer_head *sb_bh;
>>>>> int ret = -EINVAL;
>>>>> u64 devid;
>>>>> u64 transid;
>>>>> @@ -1392,8 +1392,12 @@ int btrfs_scan_one_device(const char *path,
>>>>> fmode_t flags, void *holder,
>>>>> goto error;
>>>>> }
>>>>> - if (btrfs_read_disk_super(bdev, bytenr, &page, &disk_super))
>>>>> + sb_bh = btrfs_read_dev_super(bdev);
>>>>> + if (IS_ERR(sb_bh)) {
>>>>> + ret = PTR_ERR(sb_bh);
>>>>> goto error_bdev_put;
>>>>> + }
>>>>> + disk_super = (struct btrfs_super_block *) sb_bh->b_data;
>>>>> devid = btrfs_stack_device_id(&disk_super->dev_item);
>>>>> transid = btrfs_super_generation(disk_super);
>>>>> @@ -1413,7 +1417,7 @@ int btrfs_scan_one_device(const char *path,
>>>>> fmode_t flags, void *holder,
>>>>> if (!ret && fs_devices_ret)
>>>>> (*fs_devices_ret)->total_devices = total_devices;
>>>>> - btrfs_release_disk_super(page);
>>>>> + brelse(sb_bh);
>>>>> error_bdev_put:
>>>>> blkdev_put(bdev, flags);
>>>>>
>>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe
>>> linux-btrfs" in
>>> the body of a message to majord...@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
signature.asc
Description: OpenPGP digital signature
