Re: [PATCH RFC] btrfs: self heal from SB fail

[Hans van Kranenburg <hans.van.kranenburg@mendix.com>]

On 12/08/2017 09:17 AM, 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.

That's about using 'backup roots' with old info about previous
generations than the current one on disk, which is a different thing
than using one of the other copies of the super block itself.

> 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.
> 
> Thanks,
> Qu
> 
> 
>>
>> Signed-off-by: Anand Jain <anand.jain@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);
>>
> 


-- 
Hans van Kranenburg