All of lore.kernel.org
 help / color / mirror / Atom feed
From: Carlos Maiolino <cmaiolino@redhat.com>
To: Dave Chinner <david@fromorbit.com>
Cc: "Darrick J. Wong" <darrick.wong@oracle.com>, linux-xfs@vger.kernel.org
Subject: Re: [PATCH] Cleanup old XFS_BTREE_* traces
Date: Wed, 14 Feb 2018 14:52:00 +0100	[thread overview]
Message-ID: <20180214135200.krlhiakaevsh5hi5@odin.usersys.redhat.com> (raw)
In-Reply-To: <20180213235814.GH6778@dastard>

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

  reply	other threads:[~2018-02-14 13:52 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-12 13:00 [PATCH] Cleanup old XFS_BTREE_* traces Carlos Maiolino
2018-02-12 18:29 ` Darrick J. Wong
2018-02-12 21:21   ` Dave Chinner
2018-02-13 23:17     ` Darrick J. Wong
2018-02-13 23:58       ` Dave Chinner
2018-02-14 13:52         ` Carlos Maiolino [this message]
2018-02-14 17:28           ` Darrick J. Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180214135200.krlhiakaevsh5hi5@odin.usersys.redhat.com \
    --to=cmaiolino@redhat.com \
    --cc=darrick.wong@oracle.com \
    --cc=david@fromorbit.com \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.