[dm-devel] [PATCH 21/24] dm cache: add trc policy shim

[Steven Rostedt]

On Fri, 2013-10-25 at 18:44 -0400, Mike Snitzer wrote:
> On Fri, Oct 25 2013 at  5:08pm -0400,
> Mike Snitzer <snitzer at redhat.com> wrote:
> 
> > On Fri, Oct 25 2013 at  4:13pm -0400,
> > Alasdair G Kergon <agk at redhat.com> wrote:
> > 
> > > On Thu, Oct 24, 2013 at 02:30:34PM -0400, Mike Snitzer wrote:
> > > > From: Morgan Mears <morgan.mears at netapp.com>
> > >  
> > > > This commit includes a non-terminal policy (aka "shim") called trc that
> > > > may be stacked ontop of a terminal policy (e.g. mq).
> > > > The second shim, trc, adds function-call-level tracing to the policy
> > > > stack.  By default, an INFO-level log message including function name
> > > > and parameter(s) will be generated whenever most of the cache policy
> > > > interface functions are called.  An interface to increase or decrease
> > > > the verbosity of the trace output is also provided.
> > >  
> > > Firstly, why not call it 'trace' in full, rather than abbreviating it to 3
> > > consonants?
> > 
> > We can easily rename.
> >  
> > > > +++ b/drivers/md/dm-cache-policy-trc.c
> > > 
> > > > +#define DM_TRC_OUT(lev, p, f, arg...) \
> > > > +	do { \
> > > > +		if (to_trc_policy(p)->trace_level >= lev) \
> > > > +			DMINFO("%s: " f, __func__, ## arg); \
> > > > +	} while (0)
> > > 
> > > OK for private debugging, but I can't pretend to be very keen on this
> > > one going upstream in this form.   Might this not need to support high
> > > volumes of messages sometimes?  Were other upstream mechanisms
> > > considered?  (E.g. see Documentation/trace).
> > 
> > Think this would take care of it (not full-blown tracepoints like
> > blktrace but certainly more performant than standard printk):

What's wrong with full blown tracepoints?

> 
> I folded this trace_printk change in and renamed from trc to trace, see:
> https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=for-next&id=f6f5db50495b16ff23a0febf38f9ee9d964b12dd
> 

Please please please DO NOT USE TRACE_PRINTK!

It's a debugging tool, and should never be used in the kernel proper. It
was meant for users to use it for debugging and then strip it out.

Now what you could do is to set up tracepoints to record the information
you want, either in the function itself, or add the wrapper function too
as a separate policy. Not sure if that would be any help or not.

But unless this is something that is considered "debug use only" and
never put into a production kernel, do not use trace_printk().

Note, tracepoints will also give you the ability to pick and choose the
traces instead of doing it by trace_level.

Thanks,

-- Steve