On Wed, Mar 13, 2019 at 04:55:06PM +0800, Qu Wenruo wrote: > We already have btrfs_check_chunk_valid() to verify each chunk before > tree-checker. > > Merge that function into tree-checker, and update its error message to > be more readable. > > Old error message would be something like: > BTRFS error (device dm-3): invalid chunk num_stipres: 0 > > New error message would be: > Btrfs critical (device dm-3): corrupt superblock syschunk array: chunk_start=2097152, invalid chunk num_stripes: 0 > Or > Btrfs critical (device dm-3): corrupt leaf: root=3 block=8388608 slot=3 chunk_start=2097152, invalid chunk num_stripes: 0 > > Btrfs_check_chunk_valid() is exported for super block syschunk array > verification. > > Also make tree-checker to verify chunk items, this makes chunk item > checker covers all chunks and avoid fuzzed image. > > Reported-by: Yoon Jungyeon <jungyeon@xxxxxxxxxx> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=202751 > Signed-off-by: Qu Wenruo <wqu@xxxxxxxx> > --- > fs/btrfs/tree-checker.c | 152 ++++++++++++++++++++++++++++++++++++++++ > fs/btrfs/tree-checker.h | 3 + > fs/btrfs/volumes.c | 94 +------------------------ > 3 files changed, 156 insertions(+), 93 deletions(-) Please split the patch to part where you just move the code and where the logic is changed. btrfs_check_chunk_valid is not the same, old has -EIO and new -EUCLEAN. Moving a function is ok in the same patch if there's no change. > > diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c > index b8cdaf472031..fe3f37c23c29 100644 > --- a/fs/btrfs/tree-checker.c > +++ b/fs/btrfs/tree-checker.c > @@ -448,6 +448,152 @@ static int check_block_group_item(struct btrfs_fs_info *fs_info, > return 0; > } > > +__printf(5, 6) > +__cold > +static void chunk_err(const struct btrfs_fs_info *fs_info, > + const struct extent_buffer *leaf, > + const struct btrfs_chunk *chunk, u64 logical, > + const char *fmt, ...) > +{ > + /* Only superblock eb is able to have such small offset */ > + bool is_sb = (leaf->start == BTRFS_SUPER_INFO_OFFSET); Please move the initialization and comment out of the declaration block > + struct va_format vaf; > + va_list args; > + int i; > + int slot = -1; > + > + if (!is_sb) { > + /* > + * Get the slot number by iterating through all slots, this > + * would provide better readability. > + */ > + for (i = 0; i < btrfs_header_nritems(leaf); i++) { > + if (btrfs_item_ptr_offset(leaf, i) == > + (unsigned long) chunk) { > + slot = i; > + break; > + } > + } > + }
