[v3,2/3] btrfs: Add trace point for qgroup reserved space
diff mbox

Message ID 20161108022044.686-2-quwenruo@cn.fujitsu.com
State New
Headers show

Commit Message

Qu Wenruo Nov. 8, 2016, 2:20 a.m. UTC
Introduce the following trace points:
qgroup_update_reserve
qgroup_meta_reserve

These trace points are handy to trace qgroup reserve space related
problems.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
Although I hope these trace points will not be used again.

v2:
  None
v3:
  Separate from trace point timing modification patch.
---
 fs/btrfs/qgroup.c            | 15 +++++++++++++++
 include/trace/events/btrfs.h | 43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+)

Comments

Sanidhya Solanki Nov. 8, 2016, 11:27 a.m. UTC | #1
On Tue,  8 Nov 2016 10:20:43 +0800
Qu Wenruo <quwenruo@cn.fujitsu.com> wrote:

> Introduce the following trace points:
> qgroup_update_reserve
> qgroup_meta_reserve
> 
> These trace points are handy to trace qgroup reserve space related
> problems.
> 
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> ---
> Although I hope these trace points will not be used again.
> 
> v2:
>   None
> v3:
>   Separate from trace point timing modification patch.
> ---
>  fs/btrfs/qgroup.c            | 15 +++++++++++++++
>  include/trace/events/btrfs.h | 43 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 58 insertions(+)
> 
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index fc0c64e..c00f962 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -1068,6 +1068,8 @@ static int __qgroup_excl_accounting(struct btrfs_fs_info *fs_info,
>  	qgroup->excl += sign * num_bytes;
>  	qgroup->excl_cmpr += sign * num_bytes;
>  	if (sign > 0) {
> +		trace_qgroup_update_reserve(fs_info, qgroup->qgroupid,
> +					    qgroup->reserved, (s64)-num_bytes);
>  		if (WARN_ON(qgroup->reserved < num_bytes))
>  			report_reserved_underflow(fs_info, qgroup,
>  						  num_bytes);
> @@ -1094,6 +1096,9 @@ static int __qgroup_excl_accounting(struct btrfs_fs_info *fs_info,
>  		WARN_ON(sign < 0 && qgroup->excl < num_bytes);
>  		qgroup->excl += sign * num_bytes;
>  		if (sign > 0) {
> +			trace_qgroup_update_reserve(fs_info, qgroup->qgroupid,
> +						    qgroup->reserved,
> +						    (s64)-num_bytes);
>  			if (WARN_ON(qgroup->reserved < num_bytes))
>  				report_reserved_underflow(fs_info, qgroup,
>  							  num_bytes);
> @@ -2178,6 +2183,8 @@ static int qgroup_reserve(struct btrfs_root *root, u64 num_bytes)
>  
>  		qg = u64_to_ptr(unode->aux);
>  
> +		trace_qgroup_update_reserve(fs_info, qg->qgroupid,
> +					    qg->reserved, num_bytes);
>  		qg->reserved += num_bytes;
>  	}
>  
> @@ -2223,6 +2230,8 @@ void btrfs_qgroup_free_refroot(struct btrfs_fs_info *fs_info,
>  
>  		qg = u64_to_ptr(unode->aux);
>  
> +		trace_qgroup_update_reserve(fs_info, qg->qgroupid,
> +					    qg->reserved, (s64)-num_bytes);
>  		if (WARN_ON(qgroup->reserved < num_bytes))
>  			report_reserved_underflow(fs_info, qgroup, num_bytes);
>  		else
> @@ -2706,6 +2715,8 @@ int btrfs_qgroup_reserve_meta(struct btrfs_root *root, int num_bytes)
>  		return 0;
>  
>  	BUG_ON(num_bytes != round_down(num_bytes, root->nodesize));
> +	trace_qgroup_meta_reserve(root->fs_info, root->objectid,
> +				  (s64)num_bytes);
>  	ret = qgroup_reserve(root, num_bytes);
>  	if (ret < 0)
>  		return ret;
> @@ -2724,6 +2735,8 @@ void btrfs_qgroup_free_meta_all(struct btrfs_root *root)
>  	reserved = atomic_xchg(&root->qgroup_meta_rsv, 0);
>  	if (reserved == 0)
>  		return;
> +	trace_qgroup_meta_reserve(root->fs_info, root->objectid,
> +				  (s64)-reserved);
>  	qgroup_free(root, reserved);
>  }
>  
> @@ -2736,6 +2749,8 @@ void btrfs_qgroup_free_meta(struct btrfs_root *root, int num_bytes)
>  	BUG_ON(num_bytes != round_down(num_bytes, root->nodesize));
>  	WARN_ON(atomic_read(&root->qgroup_meta_rsv) < num_bytes);
>  	atomic_sub(num_bytes, &root->qgroup_meta_rsv);
> +	trace_qgroup_meta_reserve(root->fs_info, root->objectid,
> +				  (s64)-num_bytes);
>  	qgroup_free(root, num_bytes);
>  }

Why is there a hyphen in some places after "(s64)" and not in others? Is that supposed to be
type casting or something else?

Sanidhya
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Sterba Nov. 25, 2016, 1:04 p.m. UTC | #2
On Tue, Nov 08, 2016 at 06:27:12AM -0500, Sanidhya Solanki wrote:
> On Tue,  8 Nov 2016 10:20:43 +0800
> Qu Wenruo <quwenruo@cn.fujitsu.com> wrote:
> 
> > Introduce the following trace points:
> > qgroup_update_reserve
> > qgroup_meta_reserve
> > 
> > These trace points are handy to trace qgroup reserve space related
> > problems.
> > 
> > Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> > ---
> > Although I hope these trace points will not be used again.
> > 
> > v2:
> >   None
> > v3:
> >   Separate from trace point timing modification patch.
> > ---
> >  fs/btrfs/qgroup.c            | 15 +++++++++++++++
> >  include/trace/events/btrfs.h | 43 +++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 58 insertions(+)
> > 
> > diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> > index fc0c64e..c00f962 100644
> > --- a/fs/btrfs/qgroup.c
> > +++ b/fs/btrfs/qgroup.c
> > @@ -1068,6 +1068,8 @@ static int __qgroup_excl_accounting(struct btrfs_fs_info *fs_info,
> >  	qgroup->excl += sign * num_bytes;
> >  	qgroup->excl_cmpr += sign * num_bytes;
> >  	if (sign > 0) {
> > +		trace_qgroup_update_reserve(fs_info, qgroup->qgroupid,
> > +					    qgroup->reserved, (s64)-num_bytes);
> >  		if (WARN_ON(qgroup->reserved < num_bytes))
> >  			report_reserved_underflow(fs_info, qgroup,
> >  						  num_bytes);
> > @@ -1094,6 +1096,9 @@ static int __qgroup_excl_accounting(struct btrfs_fs_info *fs_info,
> >  		WARN_ON(sign < 0 && qgroup->excl < num_bytes);
> >  		qgroup->excl += sign * num_bytes;
> >  		if (sign > 0) {
> > +			trace_qgroup_update_reserve(fs_info, qgroup->qgroupid,
> > +						    qgroup->reserved,
> > +						    (s64)-num_bytes);
> >  			if (WARN_ON(qgroup->reserved < num_bytes))
> >  				report_reserved_underflow(fs_info, qgroup,
> >  							  num_bytes);
> > @@ -2178,6 +2183,8 @@ static int qgroup_reserve(struct btrfs_root *root, u64 num_bytes)
> >  
> >  		qg = u64_to_ptr(unode->aux);
> >  
> > +		trace_qgroup_update_reserve(fs_info, qg->qgroupid,
> > +					    qg->reserved, num_bytes);
> >  		qg->reserved += num_bytes;
> >  	}
> >  
> > @@ -2223,6 +2230,8 @@ void btrfs_qgroup_free_refroot(struct btrfs_fs_info *fs_info,
> >  
> >  		qg = u64_to_ptr(unode->aux);
> >  
> > +		trace_qgroup_update_reserve(fs_info, qg->qgroupid,
> > +					    qg->reserved, (s64)-num_bytes);
> >  		if (WARN_ON(qgroup->reserved < num_bytes))
> >  			report_reserved_underflow(fs_info, qgroup, num_bytes);
> >  		else
> > @@ -2706,6 +2715,8 @@ int btrfs_qgroup_reserve_meta(struct btrfs_root *root, int num_bytes)
> >  		return 0;
> >  
> >  	BUG_ON(num_bytes != round_down(num_bytes, root->nodesize));
> > +	trace_qgroup_meta_reserve(root->fs_info, root->objectid,
> > +				  (s64)num_bytes);
> >  	ret = qgroup_reserve(root, num_bytes);
> >  	if (ret < 0)
> >  		return ret;
> > @@ -2724,6 +2735,8 @@ void btrfs_qgroup_free_meta_all(struct btrfs_root *root)
> >  	reserved = atomic_xchg(&root->qgroup_meta_rsv, 0);
> >  	if (reserved == 0)
> >  		return;
> > +	trace_qgroup_meta_reserve(root->fs_info, root->objectid,
> > +				  (s64)-reserved);
> >  	qgroup_free(root, reserved);
> >  }
> >  
> > @@ -2736,6 +2749,8 @@ void btrfs_qgroup_free_meta(struct btrfs_root *root, int num_bytes)
> >  	BUG_ON(num_bytes != round_down(num_bytes, root->nodesize));
> >  	WARN_ON(atomic_read(&root->qgroup_meta_rsv) < num_bytes);
> >  	atomic_sub(num_bytes, &root->qgroup_meta_rsv);
> > +	trace_qgroup_meta_reserve(root->fs_info, root->objectid,
> > +				  (s64)-num_bytes);
> >  	qgroup_free(root, num_bytes);
> >  }
> 
> Why is there a hyphen in some places after "(s64)" and not in others? Is that supposed to be
> type casting or something else?

Right, this looks strange and not clear from the context, besides that
the typecast should precede negation: -(s64)num_bytes

There's also a type mismatch in the 2nd tracepoint, where argument is
s64, the tracepoint entry is u64 and printf format is %lld.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch
diff mbox

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index fc0c64e..c00f962 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1068,6 +1068,8 @@  static int __qgroup_excl_accounting(struct btrfs_fs_info *fs_info,
 	qgroup->excl += sign * num_bytes;
 	qgroup->excl_cmpr += sign * num_bytes;
 	if (sign > 0) {
+		trace_qgroup_update_reserve(fs_info, qgroup->qgroupid,
+					    qgroup->reserved, (s64)-num_bytes);
 		if (WARN_ON(qgroup->reserved < num_bytes))
 			report_reserved_underflow(fs_info, qgroup,
 						  num_bytes);
@@ -1094,6 +1096,9 @@  static int __qgroup_excl_accounting(struct btrfs_fs_info *fs_info,
 		WARN_ON(sign < 0 && qgroup->excl < num_bytes);
 		qgroup->excl += sign * num_bytes;
 		if (sign > 0) {
+			trace_qgroup_update_reserve(fs_info, qgroup->qgroupid,
+						    qgroup->reserved,
+						    (s64)-num_bytes);
 			if (WARN_ON(qgroup->reserved < num_bytes))
 				report_reserved_underflow(fs_info, qgroup,
 							  num_bytes);
@@ -2178,6 +2183,8 @@  static int qgroup_reserve(struct btrfs_root *root, u64 num_bytes)
 
 		qg = u64_to_ptr(unode->aux);
 
+		trace_qgroup_update_reserve(fs_info, qg->qgroupid,
+					    qg->reserved, num_bytes);
 		qg->reserved += num_bytes;
 	}
 
@@ -2223,6 +2230,8 @@  void btrfs_qgroup_free_refroot(struct btrfs_fs_info *fs_info,
 
 		qg = u64_to_ptr(unode->aux);
 
+		trace_qgroup_update_reserve(fs_info, qg->qgroupid,
+					    qg->reserved, (s64)-num_bytes);
 		if (WARN_ON(qgroup->reserved < num_bytes))
 			report_reserved_underflow(fs_info, qgroup, num_bytes);
 		else
@@ -2706,6 +2715,8 @@  int btrfs_qgroup_reserve_meta(struct btrfs_root *root, int num_bytes)
 		return 0;
 
 	BUG_ON(num_bytes != round_down(num_bytes, root->nodesize));
+	trace_qgroup_meta_reserve(root->fs_info, root->objectid,
+				  (s64)num_bytes);
 	ret = qgroup_reserve(root, num_bytes);
 	if (ret < 0)
 		return ret;
@@ -2724,6 +2735,8 @@  void btrfs_qgroup_free_meta_all(struct btrfs_root *root)
 	reserved = atomic_xchg(&root->qgroup_meta_rsv, 0);
 	if (reserved == 0)
 		return;
+	trace_qgroup_meta_reserve(root->fs_info, root->objectid,
+				  (s64)-reserved);
 	qgroup_free(root, reserved);
 }
 
@@ -2736,6 +2749,8 @@  void btrfs_qgroup_free_meta(struct btrfs_root *root, int num_bytes)
 	BUG_ON(num_bytes != round_down(num_bytes, root->nodesize));
 	WARN_ON(atomic_read(&root->qgroup_meta_rsv) < num_bytes);
 	atomic_sub(num_bytes, &root->qgroup_meta_rsv);
+	trace_qgroup_meta_reserve(root->fs_info, root->objectid,
+				  (s64)-num_bytes);
 	qgroup_free(root, num_bytes);
 }
 
diff --git a/include/trace/events/btrfs.h b/include/trace/events/btrfs.h
index e030d6f..fb3cb6c 100644
--- a/include/trace/events/btrfs.h
+++ b/include/trace/events/btrfs.h
@@ -1468,6 +1468,49 @@  TRACE_EVENT(qgroup_update_counters,
 		  __entry->cur_new_count)
 );
 
+TRACE_EVENT(qgroup_update_reserve,
+
+	TP_PROTO(struct btrfs_fs_info *fs_info, u64 qgid, u64 cur_reserved,
+		 s64 diff),
+
+	TP_ARGS(fs_info, qgid, cur_reserved, diff),
+
+	TP_STRUCT__entry_btrfs(
+		__field(	u64,	qgid			)
+		__field(	u64,	cur_reserved		)
+		__field(	s64,	diff			)
+	),
+
+	TP_fast_assign_btrfs(fs_info,
+		__entry->qgid		= qgid;
+		__entry->cur_reserved	= cur_reserved;
+		__entry->diff		= diff;
+	),
+
+	TP_printk_btrfs("qgid = %llu, cur_reserved = %llu, diff = %lld",
+		__entry->qgid, __entry->cur_reserved, __entry->diff)
+);
+
+TRACE_EVENT(qgroup_meta_reserve,
+
+	TP_PROTO(struct btrfs_fs_info *fs_info, u64 refroot, s64 diff),
+
+	TP_ARGS(fs_info, refroot, diff),
+
+	TP_STRUCT__entry_btrfs(
+		__field(	u64,	refroot			)
+		__field(	u64,	diff			)
+	),
+
+	TP_fast_assign_btrfs(fs_info,
+		__entry->refroot	= refroot;
+		__entry->diff		= diff;
+	),
+
+	TP_printk_btrfs("refroot = %llu, diff = %lld",
+		__entry->refroot, __entry->diff)
+);
+
 #endif /* _TRACE_BTRFS_H */
 
 /* This part must be outside protection */