From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from userp2130.oracle.com ([156.151.31.86]:39206 "EHLO userp2130.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1161187AbeBNR21 (ORCPT ); Wed, 14 Feb 2018 12:28:27 -0500 Date: Wed, 14 Feb 2018 09:28:23 -0800 From: "Darrick J. Wong" Subject: Re: [PATCH] Cleanup old XFS_BTREE_* traces Message-ID: <20180214172823.GL5217@magnolia> References: <20180212130005.7199-1-cmaiolino@redhat.com> <20180212182948.GB5217@magnolia> <20180212212106.GD6778@dastard> <20180213231731.GF5217@magnolia> <20180213235814.GH6778@dastard> <20180214135200.krlhiakaevsh5hi5@odin.usersys.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180214135200.krlhiakaevsh5hi5@odin.usersys.redhat.com> Sender: linux-xfs-owner@vger.kernel.org List-ID: List-Id: xfs To: Dave Chinner , linux-xfs@vger.kernel.org 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 > > > > > > --- > > > > > > > > > > > > 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