Re: [PATCH v4 4/4] btrfs: ctree: Checking key orders before merged tree blocks

David Sterba
2020-08-14T07:59:43Z

On Fri, Aug 14, 2020 at 08:54:27AM +0800, Qu Wenruo wrote:
> On 2020/8/13 下午10:21, David Sterba wrote:
> > On Wed, Aug 12, 2020 at 02:05:09PM +0800, Qu Wenruo wrote:
> [...]
> >> ---
> >> fs/btrfs/ctree.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++
> >> 1 file changed, 68 insertions(+)
> >>
> >> diff --git a/fs/btrfs/ctree.c b/fs/btrfs/ctree.c
> >> index 70e49d8d4f6c..497abb397ea1 100644
> >> --- a/fs/btrfs/ctree.c
> >> +++ b/fs/btrfs/ctree.c
> >> @@ -3159,6 +3159,52 @@ void btrfs_set_item_key_safe(struct btrfs_fs_info *fs_info,
> >> fixup_low_keys(path, &disk_key, 1);
> >> }
> >>
> >> +/*
> >> + * Check the cross tree block key ordering.
> >> + *
> >> + * Tree-checker only works inside one tree block, thus the following
> >> + * corruption can not be rejected by tree-checker:
> >> + * Leaf @left | Leaf @right
> >> + * --------------------------------------------------------------
> >> + * | 1 | 2 | 3 | 4 | 5 | f6 | | 7 | 8 |
> >> + *
> >> + * Key f6 in leaf @left itself is valid, but not valid when the next
> >> + * key in leaf @right is 7.
> >> + * This can only be checked at tree block merge time.
> >> + * And since tree checker has ensured all key order in each tree block
> >> + * is correct, we only need to bother the last key of @left and the first
> >> + * key of @right.
> >> + */
> >> +static bool valid_cross_tree_key_order(struct extent_buffer *left,
> >> + struct extent_buffer *right)
> >
> > I think this naming is confusing, my first thought was that keys from
> > two trees were being checked, but this is for two leaves in the same
> > tree.
> >
> > The arguments should be constified.
> >
> > Elsewhere we use a check_<something> naming scheme with return value
> > true - problem, and 0/false - all ok. The 'valid' is the reverse and
> > also not following the scheme.
>
> Any good candidate?
>
> My current top list candidate is, check_sibling_keys().

That sounds perfect.