On 2021-02-19 16:20, Wang Yugui wrote:
Hi,

On 2021-02-19 04:17, Wang Yugui wrote:
Hi, Josef Bacik

We noticed an error in 5.10.x backport of 'btrfs: fix possible free
space tree corruption with online conversion'

It is wrong in 5.10.13, but right in 5.11.

5.10.13
@@ -146,6 +146,9 @@ enum {
        BTRFS_FS_STATE_DEV_REPLACING,
        /* The btrfs_fs_info created for self-tests */
        BTRFS_FS_STATE_DUMMY_FS_INFO,
+
+       /* Indicate that we can't trust the free space tree for caching yet */
+       BTRFS_FS_FREE_SPACE_TREE_UNTRUSTED,
   };

the usage sample of this enum:
set_bit(BTRFS_FS_STATE_DUMMY_FS_INFO, &fs_info->fs_state);


5.11
enum{
..
      /* Indicate that the discard workqueue can service discards. */
      BTRFS_FS_DISCARD_RUNNING,

      /* Indicate that we need to cleanup space cache v1 */
      BTRFS_FS_CLEANUP_SPACE_CACHE_V1,

      /* Indicate that we can't trust the free space tree for caching yet */
      BTRFS_FS_FREE_SPACE_TREE_UNTRUSTED,
};

the usage sample of this enum:
set_bit(BTRFS_FS_FREE_SPACE_TREE_UNTRUSTED, &fs_info->flags);

Out of curiosity I decided to check how this happened, but don't see it.
Here is the commit that went into 5.10.13 and it looks correct to me:

https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-5.10.y&id=2175bf57dc9522c58d93dcd474758434a3f05c57

The patch that went into 5.10 looks identical to the original commit in 5.11.
What tree are you looking at?

the 5.10.y is the URL that you point out.
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-5.10.y&id=2175bf57dc9522c58d93dcd474758434a3f05c57

but the right one for 5.11 is
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/fs/btrfs?id=2f96e40212d435b328459ba6b3956395eed8fa9f

5.11:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/fs/btrfs?id=2f96e40212d435b328459ba6b3956395eed8fa9f

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 0225c5208f44c..47ca8edafb5e6 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -564,6 +564,9 @@ enum {
/* Indicate that we need to cleanup space cache v1 */
        BTRFS_FS_CLEANUP_SPACE_CACHE_V1,
+
+       /* Indicate that we can't trust the free space tree for caching yet */
+       BTRFS_FS_FREE_SPACE_TREE_UNTRUSTED,
  };
/*

but 5.10.y:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-5.10.y&id=2175bf57dc9522c58d93dcd474758434a3f05c57

diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index e01545538e07f..30ea9780725ff 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -146,6 +146,9 @@ enum {
        BTRFS_FS_STATE_DEV_REPLACING,
        /* The btrfs_fs_info created for self-tests */
        BTRFS_FS_STATE_DUMMY_FS_INFO,
+
+       /* Indicate that we can't trust the free space tree for caching yet */
+       BTRFS_FS_FREE_SPACE_TREE_UNTRUSTED,
  };
#define BTRFS_BACKREF_REV_MAX 256

Both the line(Line:146 vs Line:564) and the content are wrong.


Ahh..now I understand, indeed the merge of BTRFS_FS_FREE_SPACE_TREE_UNTRUSTED 
went into
the wrong enum. I misunderstood your original posting to mean that it had 
somehow missed
a chunk or used the wrong enum value in set_bit.

Anyway, good catch! I guess Dave needs to decide how to fix this, maybe
let Greg revert & re-apply properly.

Can anybody explain why git decided to do this?

-h

Reply via email to