Re: [PATCH] btrfs: trace: Remove unnecessary fs_info parameter for btrfs__reserve_extent event class

Nikolay Borisov
2018-04-26T06:27:14Z

On 26.04.2018 09:24, Qu Wenruo wrote:
> fs_info can be extracted from btrfs_block_group_cache, and all
> btrfs_block_group_cache is created by btrfs_create_block_group_cache()
> with fs_info initialized, no need to worry about NULL pointer
> dereference.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>

I very much hate function which take parameters solely for the purpose
of trace points!

Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> ---
> fs/btrfs/extent-tree.c | 8 ++++----
> include/trace/events/btrfs.h | 18 ++++++++----------
> 2 files changed, 12 insertions(+), 14 deletions(-)
>
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 09d6e4544d6e..bf30be868f7a 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -7664,7 +7664,7 @@ static noinline int find_free_extent(struct btrfs_fs_info *fs_info,
> if (offset) {
> /* we have a block, we're done */
> spin_unlock(&last_ptr->refill_lock);
> - trace_btrfs_reserve_extent_cluster(fs_info,
> + trace_btrfs_reserve_extent_cluster(
> used_block_group,
> search_start, num_bytes);
> if (used_block_group != block_group) {
> @@ -7737,7 +7737,7 @@ static noinline int find_free_extent(struct btrfs_fs_info *fs_info,
> if (offset) {
> /* we found one, proceed */
> spin_unlock(&last_ptr->refill_lock);
> - trace_btrfs_reserve_extent_cluster(fs_info,
> + trace_btrfs_reserve_extent_cluster(
> block_group, search_start,
> num_bytes);
> goto checks;
> @@ -7837,8 +7837,8 @@ static noinline int find_free_extent(struct btrfs_fs_info *fs_info,
> ins->objectid = search_start;
> ins->offset = num_bytes;
>
> - trace_btrfs_reserve_extent(fs_info, block_group,
> - search_start, num_bytes);
> + trace_btrfs_reserve_extent(block_group, search_start,
> + num_bytes);
> btrfs_release_block_group(block_group, delalloc);
> break;
> loop:
> diff --git a/include/trace/events/btrfs.h b/include/trace/events/btrfs.h
> index d78d8ab4bc86..5af2479e9b16 100644
> --- a/include/trace/events/btrfs.h
> +++ b/include/trace/events/btrfs.h
> @@ -1146,11 +1146,10 @@ TRACE_EVENT(find_free_extent,
>
> DECLARE_EVENT_CLASS(btrfs__reserve_extent,
>
> - TP_PROTO(const struct btrfs_fs_info *fs_info,
> - const struct btrfs_block_group_cache *block_group, u64 start,
> + TP_PROTO(const struct btrfs_block_group_cache *block_group, u64 start,
> u64 len),
>
> - TP_ARGS(fs_info, block_group, start, len),
> + TP_ARGS(block_group, start, len),
>
> TP_STRUCT__entry_btrfs(
> __field( u64, bg_objectid )
> @@ -1159,7 +1158,7 @@ DECLARE_EVENT_CLASS(btrfs__reserve_extent,
> __field( u64, len )
> ),
>
> - TP_fast_assign_btrfs(fs_info,
> + TP_fast_assign_btrfs(block_group->fs_info,
> __entry->bg_objectid = block_group->key.objectid;
> __entry->flags = block_group->flags;
> __entry->start = start;
> @@ -1177,20 +1176,18 @@ DECLARE_EVENT_CLASS(btrfs__reserve_extent,
>
> DEFINE_EVENT(btrfs__reserve_extent, btrfs_reserve_extent,
>
> - TP_PROTO(const struct btrfs_fs_info *fs_info,
> - const struct btrfs_block_group_cache *block_group, u64 start,
> + TP_PROTO(const struct btrfs_block_group_cache *block_group, u64 start,
> u64 len),
>
> - TP_ARGS(fs_info, block_group, start, len)
> + TP_ARGS(block_group, start, len)
> );
>
> DEFINE_EVENT(btrfs__reserve_extent, btrfs_reserve_extent_cluster,
>
> - TP_PROTO(const struct btrfs_fs_info *fs_info,
> - const struct btrfs_block_group_cache *block_group, u64 start,
> + TP_PROTO(const struct btrfs_block_group_cache *block_group, u64 start,
> u64 len),
>
> - TP_ARGS(fs_info, block_group, start, len)
> + TP_ARGS(block_group, start, len)
> );
>
> TRACE_EVENT(btrfs_find_cluster,
> @@ -1807,6 +1804,7 @@ TRACE_EVENT(btrfs_inode_mod_outstanding_extents,
> show_root_type(__entry->root_objectid),
> __entry->ino, __entry->mod)
> );
> +
> #endif /* _TRACE_BTRFS_H */
>
> /* This part must be outside protection */
>