Message ID | 20191016073149.23244-1-wqu@suse.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | btrfs: qgroup: Fix wrong parameter order for trace events | expand |
On 16.10.19 г. 10:31 ч., Qu Wenruo wrote: > [BUG] > For btrfs:qgroup_meta_reserve event, the trace event can output garbage: > qgroup_meta_reserve: 9c7f6acc-b342-4037-bc47-7f6e4d2232d7: refroot=5(FS_TREE) type=DATA diff=2 > qgroup_meta_reserve: 9c7f6acc-b342-4037-bc47-7f6e4d2232d7: refroot=5(FS_TREE) type=0x258792 diff=2 > > Since we're in qgroup_meta_reserve() trace event, the @type should never > be DATA, while diff must be aligned to sectorsize (4K in this case). > > Only UUID and refroot is correct. > > [CAUSE] > There are two causes for this bug: > > - Bad parameter order > For trace event btrfs:qgroup_meta_reserve, we're passing wrong > parameters. > > The correct parameters are: > struct btrfs_root, s64 diff, int type. > > However the used order is: > struct btrfs_root, int type, s64 diff. > > - @type is not even assigned > What I was doing !? /facepalm > > [FIX] > Fix the super stupid bug. > > Now everything works fine: > qgroup_meta_reserve: 0477ad60-9aeb-4040-8a03-1900844d46ba: refroot=5(FS_TREE) type=META_PERTRANS diff=81920 > qgroup_meta_reserve: 0477ad60-9aeb-4040-8a03-1900844d46ba: refroot=5(FS_TREE) type=META_PREALLOC diff=16384 > qgroup_meta_reserve: 0477ad60-9aeb-4040-8a03-1900844d46ba: refroot=5(FS_TREE) type=META_PREALLOC diff=0 > qgroup_meta_reserve: 0477ad60-9aeb-4040-8a03-1900844d46ba: refroot=5(FS_TREE) type=META_PREALLOC diff=16384 > qgroup_meta_reserve: 0477ad60-9aeb-4040-8a03-1900844d46ba: refroot=5(FS_TREE) type=META_PREALLOC diff=-16384 > qgroup_meta_reserve: 0477ad60-9aeb-4040-8a03-1900844d46ba: refroot=5(FS_TREE) type=META_PREALLOC diff=16384 > qgroup_meta_reserve: 0477ad60-9aeb-4040-8a03-1900844d46ba: refroot=5(FS_TREE) type=META_PREALLOC diff=-16384 > qgroup_meta_reserve: 0477ad60-9aeb-4040-8a03-1900844d46ba: refroot=5(FS_TREE) type=META_PREALLOC diff=16384 > qgroup_meta_reserve: 0477ad60-9aeb-4040-8a03-1900844d46ba: refroot=5(FS_TREE) type=META_PREALLOC diff=-16384 > > Fixes: 4ee0d8832c2e ("btrfs: qgroup: Update trace events for metadata reservation") > Signed-off-by: Qu Wenruo <wqu@suse.com> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
On Wed, Oct 16, 2019 at 03:31:49PM +0800, Qu Wenruo wrote: > --- a/fs/btrfs/qgroup.c > +++ b/fs/btrfs/qgroup.c > @@ -3629,7 +3629,7 @@ int __btrfs_qgroup_reserve_meta(struct btrfs_root *root, int num_bytes, > return 0; > > BUG_ON(num_bytes != round_down(num_bytes, fs_info->nodesize)); > - trace_qgroup_meta_reserve(root, type, (s64)num_bytes); > + trace_qgroup_meta_reserve(root, (s64)num_bytes, type); > ret = qgroup_reserve(root, num_bytes, enforce, type); > if (ret < 0) > return ret; > @@ -3676,7 +3676,7 @@ void __btrfs_qgroup_free_meta(struct btrfs_root *root, int num_bytes, > */ > num_bytes = sub_root_meta_rsv(root, num_bytes, type); > BUG_ON(num_bytes != round_down(num_bytes, fs_info->nodesize)); > - trace_qgroup_meta_reserve(root, type, -(s64)num_bytes); > + trace_qgroup_meta_reserve(root, -(s64)num_bytes, type); > btrfs_qgroup_free_refroot(fs_info, root->root_key.objectid, > num_bytes, type); > } It would be good to split the changes, one is swapping the order and the other fixing the uninitialized type. > diff --git a/include/trace/events/btrfs.h b/include/trace/events/btrfs.h > index 5df604de4f11..0ebcaa153f93 100644 > --- a/include/trace/events/btrfs.h > +++ b/include/trace/events/btrfs.h > @@ -1710,6 +1710,7 @@ TRACE_EVENT(qgroup_meta_reserve, > TP_fast_assign_btrfs(root->fs_info, > __entry->refroot = root->root_key.objectid; > __entry->diff = diff; > + __entry->type = type; And there's more, missing type assignment in qgroup_update_reserve, redundant entry::type in qgroup_meta_convert. I've briefly checked more tracepoint initialization, seems ok. It really helps to have the same order for definition and for the assignments.
diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c index c4bb69941c77..3ad151655eb8 100644 --- a/fs/btrfs/qgroup.c +++ b/fs/btrfs/qgroup.c @@ -3629,7 +3629,7 @@ int __btrfs_qgroup_reserve_meta(struct btrfs_root *root, int num_bytes, return 0; BUG_ON(num_bytes != round_down(num_bytes, fs_info->nodesize)); - trace_qgroup_meta_reserve(root, type, (s64)num_bytes); + trace_qgroup_meta_reserve(root, (s64)num_bytes, type); ret = qgroup_reserve(root, num_bytes, enforce, type); if (ret < 0) return ret; @@ -3676,7 +3676,7 @@ void __btrfs_qgroup_free_meta(struct btrfs_root *root, int num_bytes, */ num_bytes = sub_root_meta_rsv(root, num_bytes, type); BUG_ON(num_bytes != round_down(num_bytes, fs_info->nodesize)); - trace_qgroup_meta_reserve(root, type, -(s64)num_bytes); + trace_qgroup_meta_reserve(root, -(s64)num_bytes, type); btrfs_qgroup_free_refroot(fs_info, root->root_key.objectid, num_bytes, type); } diff --git a/include/trace/events/btrfs.h b/include/trace/events/btrfs.h index 5df604de4f11..0ebcaa153f93 100644 --- a/include/trace/events/btrfs.h +++ b/include/trace/events/btrfs.h @@ -1710,6 +1710,7 @@ TRACE_EVENT(qgroup_meta_reserve, TP_fast_assign_btrfs(root->fs_info, __entry->refroot = root->root_key.objectid; __entry->diff = diff; + __entry->type = type; ), TP_printk_btrfs("refroot=%llu(%s) type=%s diff=%lld",
[BUG] For btrfs:qgroup_meta_reserve event, the trace event can output garbage: qgroup_meta_reserve: 9c7f6acc-b342-4037-bc47-7f6e4d2232d7: refroot=5(FS_TREE) type=DATA diff=2 qgroup_meta_reserve: 9c7f6acc-b342-4037-bc47-7f6e4d2232d7: refroot=5(FS_TREE) type=0x258792 diff=2 Since we're in qgroup_meta_reserve() trace event, the @type should never be DATA, while diff must be aligned to sectorsize (4K in this case). Only UUID and refroot is correct. [CAUSE] There are two causes for this bug: - Bad parameter order For trace event btrfs:qgroup_meta_reserve, we're passing wrong parameters. The correct parameters are: struct btrfs_root, s64 diff, int type. However the used order is: struct btrfs_root, int type, s64 diff. - @type is not even assigned What I was doing !? /facepalm [FIX] Fix the super stupid bug. Now everything works fine: qgroup_meta_reserve: 0477ad60-9aeb-4040-8a03-1900844d46ba: refroot=5(FS_TREE) type=META_PERTRANS diff=81920 qgroup_meta_reserve: 0477ad60-9aeb-4040-8a03-1900844d46ba: refroot=5(FS_TREE) type=META_PREALLOC diff=16384 qgroup_meta_reserve: 0477ad60-9aeb-4040-8a03-1900844d46ba: refroot=5(FS_TREE) type=META_PREALLOC diff=0 qgroup_meta_reserve: 0477ad60-9aeb-4040-8a03-1900844d46ba: refroot=5(FS_TREE) type=META_PREALLOC diff=16384 qgroup_meta_reserve: 0477ad60-9aeb-4040-8a03-1900844d46ba: refroot=5(FS_TREE) type=META_PREALLOC diff=-16384 qgroup_meta_reserve: 0477ad60-9aeb-4040-8a03-1900844d46ba: refroot=5(FS_TREE) type=META_PREALLOC diff=16384 qgroup_meta_reserve: 0477ad60-9aeb-4040-8a03-1900844d46ba: refroot=5(FS_TREE) type=META_PREALLOC diff=-16384 qgroup_meta_reserve: 0477ad60-9aeb-4040-8a03-1900844d46ba: refroot=5(FS_TREE) type=META_PREALLOC diff=16384 qgroup_meta_reserve: 0477ad60-9aeb-4040-8a03-1900844d46ba: refroot=5(FS_TREE) type=META_PREALLOC diff=-16384 Fixes: 4ee0d8832c2e ("btrfs: qgroup: Update trace events for metadata reservation") Signed-off-by: Qu Wenruo <wqu@suse.com> --- fs/btrfs/qgroup.c | 4 ++-- include/trace/events/btrfs.h | 1 + 2 files changed, 3 insertions(+), 2 deletions(-)