[snip]
>>
>> Reported-by: Leonard Lausen <leon...@lausen.nl>
>> Signed-off-by: Qu Wenruo <w...@suse.com>
>> ---
>>  fs/btrfs/disk-io.c      | 10 ++++++++++
>>  fs/btrfs/tree-checker.c | 24 +++++++++++++++++++++---
>>  fs/btrfs/tree-checker.h |  8 ++++++++
>>  3 files changed, 39 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index 6052ab508f84..fff789f8db63 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -313,6 +313,16 @@ static int csum_tree_block(struct btrfs_fs_info 
>> *fs_info,
>>                      return -EUCLEAN;
>>              }
>>      } else {
>> +            if (btrfs_header_level(buf))
>> +                    err = btrfs_check_node(fs_info, buf);
>> +            else
>> +                    err = btrfs_check_leaf_write(fs_info, buf);
>> +            if (err < 0) {
>> +                    btrfs_err(fs_info,
>> +                    "block=%llu write time tree block corruption detected",
>> +                              buf->start);
>> +                    return err;
>> +            }
> 
> This code should be moved in csum_dirty_buffer. Currently there is
> pending cleanups in csum_tree_block and the final if there will be
> removed and respective read/write code factored out in
> csum_dirty_buffer/btree_readpage_end_io_hook.

I have no preference here.
As long as the timing isn't changed, I'm fine either way.

But at least, from my last check on misc-next, there is no conflict at
all. So the pending cleanup isn't so pending right now?

Thanks,
Qu

> 
> Eventually csum_tree_block's sole purpose should be to calculate the
> checksum and nothing more.
> 
>>              write_extent_buffer(buf, result, 0, csum_size);
>>      }
>>  
>> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
>> index a62e1e837a89..b8cdaf472031 100644
>> --- a/fs/btrfs/tree-checker.c
>> +++ b/fs/btrfs/tree-checker.c
>> @@ -477,7 +477,7 @@ static int check_leaf_item(struct btrfs_fs_info *fs_info,
>>  }
>>  
>>  static int check_leaf(struct btrfs_fs_info *fs_info, struct extent_buffer 
>> *leaf,
>> -                  bool check_item_data)
>> +                  bool check_item_data, bool check_empty_leaf)
>>  {
>>      /* No valid key type is 0, so all key should be larger than this key */
>>      struct btrfs_key prev_key = {0, 0, 0};
>> @@ -516,6 +516,18 @@ static int check_leaf(struct btrfs_fs_info *fs_info, 
>> struct extent_buffer *leaf,
>>                                  owner);
>>                      return -EUCLEAN;
>>              }
>> +
>> +            /*
>> +             * Skip empty leaf check, mostly for write time tree block
>> +             *
>> +             * Such skip mostly happens for tree block write time, as
>> +             * we can't use @owner as accurate owner indicator.
>> +             * Case like balance and new tree block created for commit root
>> +             * can break owner check easily.
>> +             */
>> +            if (!check_empty_leaf)
>> +                    return 0;
>> +
>>              key.objectid = owner;
>>              key.type = BTRFS_ROOT_ITEM_KEY;
>>              key.offset = (u64)-1;
>> @@ -636,13 +648,19 @@ static int check_leaf(struct btrfs_fs_info *fs_info, 
>> struct extent_buffer *leaf,
>>  int btrfs_check_leaf_full(struct btrfs_fs_info *fs_info,
>>                        struct extent_buffer *leaf)
>>  {
>> -    return check_leaf(fs_info, leaf, true);
>> +    return check_leaf(fs_info, leaf, true, true);
>>  }
>>  
>>  int btrfs_check_leaf_relaxed(struct btrfs_fs_info *fs_info,
>>                           struct extent_buffer *leaf)
>>  {
>> -    return check_leaf(fs_info, leaf, false);
>> +    return check_leaf(fs_info, leaf, false, true);
>> +}
>> +
>> +int btrfs_check_leaf_write(struct btrfs_fs_info *fs_info,
>> +                       struct extent_buffer *leaf)
>> +{
>> +    return check_leaf(fs_info, leaf, false, false);
>>  }
>>  
>>  int btrfs_check_node(struct btrfs_fs_info *fs_info, struct extent_buffer 
>> *node)
>> diff --git a/fs/btrfs/tree-checker.h b/fs/btrfs/tree-checker.h
>> index ff043275b784..6f8d1b627c53 100644
>> --- a/fs/btrfs/tree-checker.h
>> +++ b/fs/btrfs/tree-checker.h
>> @@ -23,6 +23,14 @@ int btrfs_check_leaf_full(struct btrfs_fs_info *fs_info,
>>   */
>>  int btrfs_check_leaf_relaxed(struct btrfs_fs_info *fs_info,
>>                           struct extent_buffer *leaf);
>> +
>> +/*
>> + * Write time specific leaf checker.
>> + * Don't check if the empty leaf belongs to a tree root. Mostly for balance
>> + * and new tree created in current transaction.
>> + */
>> +int btrfs_check_leaf_write(struct btrfs_fs_info *fs_info,
>> +                       struct extent_buffer *leaf);
>>  int btrfs_check_node(struct btrfs_fs_info *fs_info, struct extent_buffer 
>> *node);
>>  
>>  #endif
>>

Reply via email to