| Message ID | 95d99a944771363259f2de25de22dffd7867d127.1663167823.git.josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: initial ctree.h cleanups, simple stuff | expand |
On 2022/9/14 23:06, Josef Bacik wrote: > This isn't used outside of inode.c, there's no reason to define it in > btrfs_inode.h. > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> Reviewed-by: Qu Wenruo <wqu@suse.com> Just a small nitpick below. > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > index 6fde13f62c1d..998d1c7134ff 100644 > --- a/fs/btrfs/inode.c > +++ b/fs/btrfs/inode.c > @@ -125,6 +125,31 @@ static struct extent_map *create_io_em(struct btrfs_inode *inode, u64 start, > u64 ram_bytes, int compress_type, > int type); > > +static inline void btrfs_print_data_csum_error(struct btrfs_inode *inode, IIRC for static function there is no need for explicit inline keyword. Under most cases the compiler should be more clever than us. Thanks, Qu > + u64 logical_start, u8 *csum, u8 *csum_expected, int mirror_num) > +{ > + struct btrfs_root *root = inode->root; > + const u32 csum_size = root->fs_info->csum_size; > + > + /* Output minus objectid, which is more meaningful */ > + if (root->root_key.objectid >= BTRFS_LAST_FREE_OBJECTID) > + btrfs_warn_rl(root->fs_info, > +"csum failed root %lld ino %lld off %llu csum " CSUM_FMT " expected csum " CSUM_FMT " mirror %d", > + root->root_key.objectid, btrfs_ino(inode), > + logical_start, > + CSUM_FMT_VALUE(csum_size, csum), > + CSUM_FMT_VALUE(csum_size, csum_expected), > + mirror_num); > + else > + btrfs_warn_rl(root->fs_info, > +"csum failed root %llu ino %llu off %llu csum " CSUM_FMT " expected csum " CSUM_FMT " mirror %d", > + root->root_key.objectid, btrfs_ino(inode), > + logical_start, > + CSUM_FMT_VALUE(csum_size, csum), > + CSUM_FMT_VALUE(csum_size, csum_expected), > + mirror_num); > +} > + > /* > * btrfs_inode_lock - lock inode i_rwsem based on arguments passed > *
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
On Thu, Sep 15, 2022 at 05:22:48PM +0800, Qu Wenruo wrote: > > > On 2022/9/14 23:06, Josef Bacik wrote: > > This isn't used outside of inode.c, there's no reason to define it in > > btrfs_inode.h. > > > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> > > Reviewed-by: Qu Wenruo <wqu@suse.com> > > Just a small nitpick below. > > > diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c > > index 6fde13f62c1d..998d1c7134ff 100644 > > --- a/fs/btrfs/inode.c > > +++ b/fs/btrfs/inode.c > > @@ -125,6 +125,31 @@ static struct extent_map *create_io_em(struct btrfs_inode *inode, u64 start, > > u64 ram_bytes, int compress_type, > > int type); > > > > +static inline void btrfs_print_data_csum_error(struct btrfs_inode *inode, > > IIRC for static function there is no need for explicit inline keyword. > > Under most cases the compiler should be more clever than us. For error printing function we don't need inlining at all and if we'd want to micro optimize the function can be put to __cold section.
diff --git a/fs/btrfs/btrfs_inode.h b/fs/btrfs/btrfs_inode.h index 108af52ba870..890c9f979a3d 100644 --- a/fs/btrfs/btrfs_inode.h +++ b/fs/btrfs/btrfs_inode.h @@ -426,30 +426,4 @@ static inline void btrfs_inode_split_flags(u64 inode_item_flags, /* Array of bytes with variable length, hexadecimal format 0x1234 */ #define CSUM_FMT "0x%*phN" #define CSUM_FMT_VALUE(size, bytes) size, bytes - -static inline void btrfs_print_data_csum_error(struct btrfs_inode *inode, - u64 logical_start, u8 *csum, u8 *csum_expected, int mirror_num) -{ - struct btrfs_root *root = inode->root; - const u32 csum_size = root->fs_info->csum_size; - - /* Output minus objectid, which is more meaningful */ - if (root->root_key.objectid >= BTRFS_LAST_FREE_OBJECTID) - btrfs_warn_rl(root->fs_info, -"csum failed root %lld ino %lld off %llu csum " CSUM_FMT " expected csum " CSUM_FMT " mirror %d", - root->root_key.objectid, btrfs_ino(inode), - logical_start, - CSUM_FMT_VALUE(csum_size, csum), - CSUM_FMT_VALUE(csum_size, csum_expected), - mirror_num); - else - btrfs_warn_rl(root->fs_info, -"csum failed root %llu ino %llu off %llu csum " CSUM_FMT " expected csum " CSUM_FMT " mirror %d", - root->root_key.objectid, btrfs_ino(inode), - logical_start, - CSUM_FMT_VALUE(csum_size, csum), - CSUM_FMT_VALUE(csum_size, csum_expected), - mirror_num); -} - #endif diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 6fde13f62c1d..998d1c7134ff 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -125,6 +125,31 @@ static struct extent_map *create_io_em(struct btrfs_inode *inode, u64 start, u64 ram_bytes, int compress_type, int type); +static inline void btrfs_print_data_csum_error(struct btrfs_inode *inode, + u64 logical_start, u8 *csum, u8 *csum_expected, int mirror_num) +{ + struct btrfs_root *root = inode->root; + const u32 csum_size = root->fs_info->csum_size; + + /* Output minus objectid, which is more meaningful */ + if (root->root_key.objectid >= BTRFS_LAST_FREE_OBJECTID) + btrfs_warn_rl(root->fs_info, +"csum failed root %lld ino %lld off %llu csum " CSUM_FMT " expected csum " CSUM_FMT " mirror %d", + root->root_key.objectid, btrfs_ino(inode), + logical_start, + CSUM_FMT_VALUE(csum_size, csum), + CSUM_FMT_VALUE(csum_size, csum_expected), + mirror_num); + else + btrfs_warn_rl(root->fs_info, +"csum failed root %llu ino %llu off %llu csum " CSUM_FMT " expected csum " CSUM_FMT " mirror %d", + root->root_key.objectid, btrfs_ino(inode), + logical_start, + CSUM_FMT_VALUE(csum_size, csum), + CSUM_FMT_VALUE(csum_size, csum_expected), + mirror_num); +} + /* * btrfs_inode_lock - lock inode i_rwsem based on arguments passed *
This isn't used outside of inode.c, there's no reason to define it in btrfs_inode.h. Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- fs/btrfs/btrfs_inode.h | 26 -------------------------- fs/btrfs/inode.c | 25 +++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 26 deletions(-)