On 12/08/2017 08:12 PM, Nikolay Borisov wrote:


On  8.12.2017 12:33, Anand Jain wrote:


On 12/08/2017 04:40 PM, Nikolay Borisov wrote:


On  8.12.2017 09: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.

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);

This patch prompts another question: why do we have a page-based and a
bufferhead-based interface to reading the super block?

  Right. we need to know that. Sorry I just saw this.

FWIW unless we explicitly need a per-block state tracking (which I don't
think we do) we should ideally switch to page-based mechanism. Buffer
heads are considered deprecated. Also the only reason why we do have
btrfsic_submit_bh is for the superblock bufer head. So potentially by
removing the only requirement in the kernel where bh are used we can
simplify the btrfsic code as well . Just something to keep in mind.

 Thanks. These are related to the other email thread [1]
 [1]
 [Question] Two variants of SB reads can we move into bio read instead ?
 We can follow up there. ? Could we ? That cleanup is due for a long
 time now.

Thanks, Anand


  I have a very old patch to converge them and clean this up, but haven't
  sent it because there are some missing information on why it ended up
  like that in the first place.

Thanks, Anand


I did prototype
switching the bufferheads to page based but the resulting code wasn't
any cleaner. I believe there is also open the question what happens when
btrfs is run on a 64k page machine. I.e. we are going to read a single
page and the sb is going to be in the first 4k but what about the rest
60, they could potentially contain other metadata. The page will have to
be freed asap so as not to peg the neighboring metadata?



+    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

--
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

Reply via email to