From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from syrinx.knorrie.org ([82.94.188.77]:53778 "EHLO syrinx.knorrie.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752481AbdLHMk7 (ORCPT ); Fri, 8 Dec 2017 07:40:59 -0500 Subject: Re: [PATCH RFC] btrfs: self heal from SB fail To: Qu Wenruo , Anand Jain , linux-btrfs@vger.kernel.org References: <20171208075705.23462-1-anand.jain@oracle.com> From: Hans van Kranenburg Message-ID: Date: Fri, 8 Dec 2017 13:40:56 +0100 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Sender: linux-btrfs-owner@vger.kernel.org List-ID: 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 >> --- >> 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