[PATCH] Cleanup old XFS_BTREE_* traces

[PATCH] Cleanup old XFS_BTREE_* traces

[Carlos Maiolino]

Remove unused legacy btree traces from IRIX era.

Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
---

Talking to Dave about it, he mentioned XFS_BTREE_TRACE_CURSOR might be worth to
turn into a proper ftrace trace point, so I didn't touch _CURSOR traces in this
patchset, and a proper conversion will be sent later, unless it's not worth at
all, and I should send a V2 also removing TRACE_CURSOR.

Cheers

 fs/xfs/libxfs/xfs_btree.c | 15 ---------------
 fs/xfs/libxfs/xfs_btree.h |  7 -------
 2 files changed, 22 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
index 79ee4a1951d1..eb324eb9e5cd 100644
--- a/fs/xfs/libxfs/xfs_btree.c
+++ b/fs/xfs/libxfs/xfs_btree.c
@@ -1439,7 +1439,6 @@ xfs_btree_log_keys(
 	int			last)
 {
 	XFS_BTREE_TRACE_CURSOR(cur, XBT_ENTRY);
-	XFS_BTREE_TRACE_ARGBII(cur, bp, first, last);
 
 	if (bp) {
 		xfs_trans_buf_set_type(cur->bc_tp, bp, XFS_BLFT_BTREE_BUF);
@@ -1465,7 +1464,6 @@ xfs_btree_log_recs(
 	int			last)
 {
 	XFS_BTREE_TRACE_CURSOR(cur, XBT_ENTRY);
-	XFS_BTREE_TRACE_ARGBII(cur, bp, first, last);
 
 	xfs_trans_buf_set_type(cur->bc_tp, bp, XFS_BLFT_BTREE_BUF);
 	xfs_trans_log_buf(cur->bc_tp, bp,
@@ -1486,7 +1484,6 @@ xfs_btree_log_ptrs(
 	int			last)	/* index of last pointer to log */
 {
 	XFS_BTREE_TRACE_CURSOR(cur, XBT_ENTRY);
-	XFS_BTREE_TRACE_ARGBII(cur, bp, first, last);
 
 	if (bp) {
 		struct xfs_btree_block	*block = XFS_BUF_TO_BLOCK(bp);
@@ -1544,7 +1541,6 @@ xfs_btree_log_block(
 	};
 
 	XFS_BTREE_TRACE_CURSOR(cur, XBT_ENTRY);
-	XFS_BTREE_TRACE_ARGBI(cur, bp, fields);
 
 	if (bp) {
 		int nbits;
@@ -1594,7 +1590,6 @@ xfs_btree_increment(
 	int			lev;
 
 	XFS_BTREE_TRACE_CURSOR(cur, XBT_ENTRY);
-	XFS_BTREE_TRACE_ARGI(cur, level);
 
 	ASSERT(level < cur->bc_nlevels);
 
@@ -1702,7 +1697,6 @@ xfs_btree_decrement(
 	union xfs_btree_ptr	ptr;
 
 	XFS_BTREE_TRACE_CURSOR(cur, XBT_ENTRY);
-	XFS_BTREE_TRACE_ARGI(cur, level);
 
 	ASSERT(level < cur->bc_nlevels);
 
@@ -1882,7 +1876,6 @@ xfs_btree_lookup(
 	union xfs_btree_ptr	ptr;	/* ptr to btree block */
 
 	XFS_BTREE_TRACE_CURSOR(cur, XBT_ENTRY);
-	XFS_BTREE_TRACE_ARGI(cur, dir);
 
 	XFS_BTREE_STATS_INC(cur, lookup);
 
@@ -2225,7 +2218,6 @@ xfs_btree_update_keys(
 		return __xfs_btree_updkeys(cur, level, block, bp, false);
 
 	XFS_BTREE_TRACE_CURSOR(cur, XBT_ENTRY);
-	XFS_BTREE_TRACE_ARGIK(cur, level, keyp);
 
 	/*
 	 * Go up the tree from this level toward the root.
@@ -2273,7 +2265,6 @@ xfs_btree_update(
 	union xfs_btree_rec	*rp;
 
 	XFS_BTREE_TRACE_CURSOR(cur, XBT_ENTRY);
-	XFS_BTREE_TRACE_ARGR(cur, rec);
 
 	/* Pick up the current block. */
 	block = xfs_btree_get_block(cur, 0, &bp);
@@ -2340,7 +2331,6 @@ xfs_btree_lshift(
 	int			i;
 
 	XFS_BTREE_TRACE_CURSOR(cur, XBT_ENTRY);
-	XFS_BTREE_TRACE_ARGI(cur, level);
 
 	if ((cur->bc_flags & XFS_BTREE_ROOT_IN_INODE) &&
 	    level == cur->bc_nlevels - 1)
@@ -2542,7 +2532,6 @@ xfs_btree_rshift(
 	int			i;		/* loop counter */
 
 	XFS_BTREE_TRACE_CURSOR(cur, XBT_ENTRY);
-	XFS_BTREE_TRACE_ARGI(cur, level);
 
 	if ((cur->bc_flags & XFS_BTREE_ROOT_IN_INODE) &&
 	    (level == cur->bc_nlevels - 1))
@@ -2727,8 +2716,6 @@ __xfs_btree_split(
 #endif
 
 	XFS_BTREE_TRACE_CURSOR(cur, XBT_ENTRY);
-	XFS_BTREE_TRACE_ARGIPK(cur, level, *ptrp, key);
-
 	XFS_BTREE_STATS_INC(cur, split);
 
 	/* Set up left block (current one). */
@@ -3310,7 +3297,6 @@ xfs_btree_insrec(
 	xfs_daddr_t		old_bn;
 
 	XFS_BTREE_TRACE_CURSOR(cur, XBT_ENTRY);
-	XFS_BTREE_TRACE_ARGIPR(cur, level, *ptrp, &rec);
 
 	ncur = NULL;
 	lkey = &nkey;
@@ -3781,7 +3767,6 @@ xfs_btree_delrec(
 	int			numrecs;	/* temporary numrec count */
 
 	XFS_BTREE_TRACE_CURSOR(cur, XBT_ENTRY);
-	XFS_BTREE_TRACE_ARGI(cur, level);
 
 	tcur = NULL;
 
diff --git a/fs/xfs/libxfs/xfs_btree.h b/fs/xfs/libxfs/xfs_btree.h
index 50440b5618e8..7d4e6b16981d 100644
--- a/fs/xfs/libxfs/xfs_btree.h
+++ b/fs/xfs/libxfs/xfs_btree.h
@@ -483,13 +483,6 @@ static inline int xfs_btree_get_level(struct xfs_btree_block *block)
  * r = btree record
  * k = btree key
  */
-#define	XFS_BTREE_TRACE_ARGBI(c, b, i)
-#define	XFS_BTREE_TRACE_ARGBII(c, b, i, j)
-#define	XFS_BTREE_TRACE_ARGI(c, i)
-#define	XFS_BTREE_TRACE_ARGIPK(c, i, p, s)
-#define	XFS_BTREE_TRACE_ARGIPR(c, i, p, r)
-#define	XFS_BTREE_TRACE_ARGIK(c, i, k)
-#define XFS_BTREE_TRACE_ARGR(c, r)
 #define	XFS_BTREE_TRACE_CURSOR(c, t)
 
 xfs_failaddr_t xfs_btree_sblock_v5hdr_verify(struct xfs_buf *bp);
-- 
2.14.3

Re: [PATCH] Cleanup old XFS_BTREE_* traces

[Darrick J. Wong]

On Mon, Feb 12, 2018 at 02:00:05PM +0100, Carlos Maiolino wrote:
> Remove unused legacy btree traces from IRIX era.
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
> 
> Talking to Dave about it, he mentioned XFS_BTREE_TRACE_CURSOR might be worth to
> turn into a proper ftrace trace point, so I didn't touch _CURSOR traces in this
> patchset, and a proper conversion will be sent later, unless it's not worth at
> all, and I should send a V2 also removing TRACE_CURSOR.

TBH I wonder the opposite -- why not turn all of these into tracepoints?
There must have been some reason for capturing the cursor state along
with an integer/ptr/record/key.  It's the XBT_ENTRY/XBT_EXIT stuff that
I think could go away, since ftrace can already record function
entry/exit.

So, all the XFS_BTREE_TRACE_ARG* become their own tracepoints each named
after the function they're in (see example below).  The XBT_ENTRY and
XBT_FXIT traces can be removed entirely; and the XBT_ERROR traces can
become a new trace_xfs_btree_cursor_error tracepoint.

> Cheers
> 
>  fs/xfs/libxfs/xfs_btree.c | 15 ---------------
>  fs/xfs/libxfs/xfs_btree.h |  7 -------
>  2 files changed, 22 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
> index 79ee4a1951d1..eb324eb9e5cd 100644
> --- a/fs/xfs/libxfs/xfs_btree.c
> +++ b/fs/xfs/libxfs/xfs_btree.c
> @@ -1439,7 +1439,6 @@ xfs_btree_log_keys(
>  	int			last)
>  {
>  	XFS_BTREE_TRACE_CURSOR(cur, XBT_ENTRY);
> -	XFS_BTREE_TRACE_ARGBII(cur, bp, first, last);

trace_xfs_btree_log_keys(cur, bp, first, last);

then eliminate the XBT_EXIT thing at the end of the function.

--D

>  
>  	if (bp) {
>  		xfs_trans_buf_set_type(cur->bc_tp, bp, XFS_BLFT_BTREE_BUF);
> @@ -1465,7 +1464,6 @@ xfs_btree_log_recs(
>  	int			last)
>  {
>  	XFS_BTREE_TRACE_CURSOR(cur, XBT_ENTRY);
> -	XFS_BTREE_TRACE_ARGBII(cur, bp, first, last);
>  
>  	xfs_trans_buf_set_type(cur->bc_tp, bp, XFS_BLFT_BTREE_BUF);
>  	xfs_trans_log_buf(cur->bc_tp, bp,
> @@ -1486,7 +1484,6 @@ xfs_btree_log_ptrs(
>  	int			last)	/* index of last pointer to log */
>  {
>  	XFS_BTREE_TRACE_CURSOR(cur, XBT_ENTRY);
> -	XFS_BTREE_TRACE_ARGBII(cur, bp, first, last);
>  
>  	if (bp) {
>  		struct xfs_btree_block	*block = XFS_BUF_TO_BLOCK(bp);
> @@ -1544,7 +1541,6 @@ xfs_btree_log_block(
>  	};
>  
>  	XFS_BTREE_TRACE_CURSOR(cur, XBT_ENTRY);
> -	XFS_BTREE_TRACE_ARGBI(cur, bp, fields);
>  
>  	if (bp) {
>  		int nbits;
> @@ -1594,7 +1590,6 @@ xfs_btree_increment(
>  	int			lev;
>  
>  	XFS_BTREE_TRACE_CURSOR(cur, XBT_ENTRY);
> -	XFS_BTREE_TRACE_ARGI(cur, level);
>  
>  	ASSERT(level < cur->bc_nlevels);
>  
> @@ -1702,7 +1697,6 @@ xfs_btree_decrement(
>  	union xfs_btree_ptr	ptr;
>  
>  	XFS_BTREE_TRACE_CURSOR(cur, XBT_ENTRY);
> -	XFS_BTREE_TRACE_ARGI(cur, level);
>  
>  	ASSERT(level < cur->bc_nlevels);
>  
> @@ -1882,7 +1876,6 @@ xfs_btree_lookup(
>  	union xfs_btree_ptr	ptr;	/* ptr to btree block */
>  
>  	XFS_BTREE_TRACE_CURSOR(cur, XBT_ENTRY);
> -	XFS_BTREE_TRACE_ARGI(cur, dir);
>  
>  	XFS_BTREE_STATS_INC(cur, lookup);
>  
> @@ -2225,7 +2218,6 @@ xfs_btree_update_keys(
>  		return __xfs_btree_updkeys(cur, level, block, bp, false);
>  
>  	XFS_BTREE_TRACE_CURSOR(cur, XBT_ENTRY);
> -	XFS_BTREE_TRACE_ARGIK(cur, level, keyp);
>  
>  	/*
>  	 * Go up the tree from this level toward the root.
> @@ -2273,7 +2265,6 @@ xfs_btree_update(
>  	union xfs_btree_rec	*rp;
>  
>  	XFS_BTREE_TRACE_CURSOR(cur, XBT_ENTRY);
> -	XFS_BTREE_TRACE_ARGR(cur, rec);
>  
>  	/* Pick up the current block. */
>  	block = xfs_btree_get_block(cur, 0, &bp);
> @@ -2340,7 +2331,6 @@ xfs_btree_lshift(
>  	int			i;
>  
>  	XFS_BTREE_TRACE_CURSOR(cur, XBT_ENTRY);
> -	XFS_BTREE_TRACE_ARGI(cur, level);
>  
>  	if ((cur->bc_flags & XFS_BTREE_ROOT_IN_INODE) &&
>  	    level == cur->bc_nlevels - 1)
> @@ -2542,7 +2532,6 @@ xfs_btree_rshift(
>  	int			i;		/* loop counter */
>  
>  	XFS_BTREE_TRACE_CURSOR(cur, XBT_ENTRY);
> -	XFS_BTREE_TRACE_ARGI(cur, level);
>  
>  	if ((cur->bc_flags & XFS_BTREE_ROOT_IN_INODE) &&
>  	    (level == cur->bc_nlevels - 1))
> @@ -2727,8 +2716,6 @@ __xfs_btree_split(
>  #endif
>  
>  	XFS_BTREE_TRACE_CURSOR(cur, XBT_ENTRY);
> -	XFS_BTREE_TRACE_ARGIPK(cur, level, *ptrp, key);
> -
>  	XFS_BTREE_STATS_INC(cur, split);
>  
>  	/* Set up left block (current one). */
> @@ -3310,7 +3297,6 @@ xfs_btree_insrec(
>  	xfs_daddr_t		old_bn;
>  
>  	XFS_BTREE_TRACE_CURSOR(cur, XBT_ENTRY);
> -	XFS_BTREE_TRACE_ARGIPR(cur, level, *ptrp, &rec);
>  
>  	ncur = NULL;
>  	lkey = &nkey;
> @@ -3781,7 +3767,6 @@ xfs_btree_delrec(
>  	int			numrecs;	/* temporary numrec count */
>  
>  	XFS_BTREE_TRACE_CURSOR(cur, XBT_ENTRY);
> -	XFS_BTREE_TRACE_ARGI(cur, level);
>  
>  	tcur = NULL;
>  
> diff --git a/fs/xfs/libxfs/xfs_btree.h b/fs/xfs/libxfs/xfs_btree.h
> index 50440b5618e8..7d4e6b16981d 100644
> --- a/fs/xfs/libxfs/xfs_btree.h
> +++ b/fs/xfs/libxfs/xfs_btree.h
> @@ -483,13 +483,6 @@ static inline int xfs_btree_get_level(struct xfs_btree_block *block)
>   * r = btree record
>   * k = btree key
>   */
> -#define	XFS_BTREE_TRACE_ARGBI(c, b, i)
> -#define	XFS_BTREE_TRACE_ARGBII(c, b, i, j)
> -#define	XFS_BTREE_TRACE_ARGI(c, i)
> -#define	XFS_BTREE_TRACE_ARGIPK(c, i, p, s)
> -#define	XFS_BTREE_TRACE_ARGIPR(c, i, p, r)
> -#define	XFS_BTREE_TRACE_ARGIK(c, i, k)
> -#define XFS_BTREE_TRACE_ARGR(c, r)
>  #define	XFS_BTREE_TRACE_CURSOR(c, t)
>  
>  xfs_failaddr_t xfs_btree_sblock_v5hdr_verify(struct xfs_buf *bp);
> -- 
> 2.14.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Cleanup old XFS_BTREE_* traces

[Dave Chinner]

On Mon, Feb 12, 2018 at 10:29:48AM -0800, Darrick J. Wong wrote:
> On Mon, Feb 12, 2018 at 02:00:05PM +0100, Carlos Maiolino wrote:
> > Remove unused legacy btree traces from IRIX era.
> > 
> > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> > ---
> > 
> > Talking to Dave about it, he mentioned XFS_BTREE_TRACE_CURSOR might be worth to
> > turn into a proper ftrace trace point, so I didn't touch _CURSOR traces in this
> > patchset, and a proper conversion will be sent later, unless it's not worth at
> > all, and I should send a V2 also removing TRACE_CURSOR.
> 
> TBH I wonder the opposite -- why not turn all of these into tracepoints?

TBH, we haven't used them in at least 15 years. What value do they
provide apart from making the traces even noisier (and potentially
more lossy) than they already are?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] Cleanup old XFS_BTREE_* traces

[Darrick J. Wong]

On Tue, Feb 13, 2018 at 08:21:06AM +1100, Dave Chinner wrote:
> On Mon, Feb 12, 2018 at 10:29:48AM -0800, Darrick J. Wong wrote:
> > On Mon, Feb 12, 2018 at 02:00:05PM +0100, Carlos Maiolino wrote:
> > > Remove unused legacy btree traces from IRIX era.
> > > 
> > > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> > > ---
> > > 
> > > Talking to Dave about it, he mentioned XFS_BTREE_TRACE_CURSOR might be worth to
> > > turn into a proper ftrace trace point, so I didn't touch _CURSOR traces in this
> > > patchset, and a proper conversion will be sent later, unless it's not worth at
> > > all, and I should send a V2 also removing TRACE_CURSOR.
> > 
> > TBH I wonder the opposite -- why not turn all of these into tracepoints?
> 
> TBH, we haven't used them in at least 15 years. What value do they
> provide apart from making the traces even noisier (and potentially
> more lossy) than they already are?

FWIW adding trace_printks to some of those functions was rather useful
for checking that the unusual refcount and rmap btree semantics actually
resulted in calls to the desired btree functions.  I wish I'd cleaned up
that debugging patch and sent it, but it's lost now.

--D

> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Cleanup old XFS_BTREE_* traces

[Dave Chinner]

On Tue, Feb 13, 2018 at 03:17:31PM -0800, Darrick J. Wong wrote:
> On Tue, Feb 13, 2018 at 08:21:06AM +1100, Dave Chinner wrote:
> > On Mon, Feb 12, 2018 at 10:29:48AM -0800, Darrick J. Wong wrote:
> > > On Mon, Feb 12, 2018 at 02:00:05PM +0100, Carlos Maiolino wrote:
> > > > Remove unused legacy btree traces from IRIX era.
> > > > 
> > > > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> > > > ---
> > > > 
> > > > Talking to Dave about it, he mentioned XFS_BTREE_TRACE_CURSOR might be worth to
> > > > turn into a proper ftrace trace point, so I didn't touch _CURSOR traces in this
> > > > patchset, and a proper conversion will be sent later, unless it's not worth at
> > > > all, and I should send a V2 also removing TRACE_CURSOR.
> > > 
> > > TBH I wonder the opposite -- why not turn all of these into tracepoints?
> > 
> > TBH, we haven't used them in at least 15 years. What value do they
> > provide apart from making the traces even noisier (and potentially
> > more lossy) than they already are?
> 
> FWIW adding trace_printks to some of those functions was rather useful
> for checking that the unusual refcount and rmap btree semantics actually
> resulted in calls to the desired btree functions.  I wish I'd cleaned up
> that debugging patch and sent it, but it's lost now.

Ok, so the non-cursor traces would have been useful to you.
That's fine - I just don't want to add stuff that doesn't have any
specific use because it's already hard enough to filter traces down
to just the things we need to see....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [PATCH] Cleanup old XFS_BTREE_* traces

[Carlos Maiolino]

On Wed, Feb 14, 2018 at 10:58:14AM +1100, Dave Chinner wrote:
> On Tue, Feb 13, 2018 at 03:17:31PM -0800, Darrick J. Wong wrote:
> > On Tue, Feb 13, 2018 at 08:21:06AM +1100, Dave Chinner wrote:
> > > On Mon, Feb 12, 2018 at 10:29:48AM -0800, Darrick J. Wong wrote:
> > > > On Mon, Feb 12, 2018 at 02:00:05PM +0100, Carlos Maiolino wrote:
> > > > > Remove unused legacy btree traces from IRIX era.
> > > > > 
> > > > > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> > > > > ---
> > > > > 
> > > > > Talking to Dave about it, he mentioned XFS_BTREE_TRACE_CURSOR might be worth to
> > > > > turn into a proper ftrace trace point, so I didn't touch _CURSOR traces in this
> > > > > patchset, and a proper conversion will be sent later, unless it's not worth at
> > > > > all, and I should send a V2 also removing TRACE_CURSOR.
> > > > 
> > > > TBH I wonder the opposite -- why not turn all of these into tracepoints?
> > > 
> > > TBH, we haven't used them in at least 15 years. What value do they
> > > provide apart from making the traces even noisier (and potentially
> > > more lossy) than they already are?
> > 
> > FWIW adding trace_printks to some of those functions was rather useful
> > for checking that the unusual refcount and rmap btree semantics actually
> > resulted in calls to the desired btree functions.  I wish I'd cleaned up
> > that debugging patch and sent it, but it's lost now.
> 

I see your point, although, for me, it sounds like a very specific debug case,
and not something which would add enough benefit to have these extra debug
traces lying around.

I mean, if we keep adding trace points for many single debug use-cases, we would
end up with tons of trace points in xfs, which are not used in 99% of the debugging
processes.

But well, I don't have a decent knowledge in the Btrees infra-structure yet to
give a more detailed reason if such extra trace points would be useful or not in
several situations. So, this is just my $0.02, based on the amount of trace
points we already have, where, most of time I use one or another, and yet, I
need to add my own trace_printks, so, particularly, I don't think adding such
extra traces in Btree code would be that useful in a long-term period, but well,
as I said, just my $0.02.

> Ok, so the non-cursor traces would have been useful to you.
> That's fine - I just don't want to add stuff that doesn't have any
> specific use because it's already hard enough to filter traces down
> to just the things we need to see....
> 

> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com

-- 
Carlos

Re: [PATCH] Cleanup old XFS_BTREE_* traces

[Darrick J. Wong]

On Wed, Feb 14, 2018 at 02:52:00PM +0100, Carlos Maiolino wrote:
> On Wed, Feb 14, 2018 at 10:58:14AM +1100, Dave Chinner wrote:
> > On Tue, Feb 13, 2018 at 03:17:31PM -0800, Darrick J. Wong wrote:
> > > On Tue, Feb 13, 2018 at 08:21:06AM +1100, Dave Chinner wrote:
> > > > On Mon, Feb 12, 2018 at 10:29:48AM -0800, Darrick J. Wong wrote:
> > > > > On Mon, Feb 12, 2018 at 02:00:05PM +0100, Carlos Maiolino wrote:
> > > > > > Remove unused legacy btree traces from IRIX era.
> > > > > > 
> > > > > > Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> > > > > > ---
> > > > > > 
> > > > > > Talking to Dave about it, he mentioned XFS_BTREE_TRACE_CURSOR might be worth to
> > > > > > turn into a proper ftrace trace point, so I didn't touch _CURSOR traces in this
> > > > > > patchset, and a proper conversion will be sent later, unless it's not worth at
> > > > > > all, and I should send a V2 also removing TRACE_CURSOR.
> > > > > 
> > > > > TBH I wonder the opposite -- why not turn all of these into tracepoints?
> > > > 
> > > > TBH, we haven't used them in at least 15 years. What value do they
> > > > provide apart from making the traces even noisier (and potentially
> > > > more lossy) than they already are?
> > > 
> > > FWIW adding trace_printks to some of those functions was rather useful
> > > for checking that the unusual refcount and rmap btree semantics actually
> > > resulted in calls to the desired btree functions.  I wish I'd cleaned up
> > > that debugging patch and sent it, but it's lost now.
> > 
> 
> I see your point, although, for me, it sounds like a very specific debug case,
> and not something which would add enough benefit to have these extra debug
> traces lying around.
> 
> I mean, if we keep adding trace points for many single debug use-cases, we would
> end up with tons of trace points in xfs, which are not used in 99% of the debugging
> processes.
> 
> But well, I don't have a decent knowledge in the Btrees infra-structure yet to
> give a more detailed reason if such extra trace points would be useful or not in
> several situations. So, this is just my $0.02, based on the amount of trace
> points we already have, where, most of time I use one or another, and yet, I
> need to add my own trace_printks, so, particularly, I don't think adding such
> extra traces in Btree code would be that useful in a long-term period, but well,
> as I said, just my $0.02.

We already have 531 tracepoints, a few more probably isn't going to
hurt.  That said, if you want to send a patch cleaning out both of the
old trace macros I'd take it.  At worst, if I or anyone else ever come
across a need to add tracepoints to the core btree code they're not hard
to add.

--D

> 
> > Ok, so the non-cursor traces would have been useful to you.
> > That's fine - I just don't want to add stuff that doesn't have any
> > specific use because it's already hard enough to filter traces down
> > to just the things we need to see....
> > 
> 
> > Cheers,
> > 
> > Dave.
> > -- 
> > Dave Chinner
> > david@fromorbit.com
> 
> -- 
> Carlos
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html