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

[Mike Snitzer <snitzer@redhat.com>]

On Fri, Oct 25 2013 at  4:13pm -0400,
Alasdair G Kergon <agk@redhat.com> wrote:

> On Thu, Oct 24, 2013 at 02:30:34PM -0400, Mike Snitzer wrote:
> > From: Morgan Mears <morgan.mears@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):

---
 drivers/md/dm-cache-policy-trc.c |    2 +-
 include/linux/device-mapper.h    |    3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

Index: linux/drivers/md/dm-cache-policy-trc.c
===================================================================
--- linux.orig/drivers/md/dm-cache-policy-trc.c
+++ linux/drivers/md/dm-cache-policy-trc.c
@@ -27,7 +27,7 @@
 #define DM_TRC_OUT(lev, p, f, arg...) \
 	do { \
 		if (to_trc_policy(p)->trace_level >= lev) \
-			DMINFO("%s: " f, __func__, ## arg); \
+			DMTRACE("%s: " f, __func__, ## arg); \
 	} while (0)
 
 enum dm_trace_lev_e {
Index: linux/include/linux/device-mapper.h
===================================================================
--- linux.orig/include/linux/device-mapper.h
+++ linux/include/linux/device-mapper.h
@@ -520,6 +520,9 @@ extern struct ratelimit_state dm_ratelim
 			       "\n", ## arg); \
 	} while (0)
 
+#define DMTRACE(f, arg...) \
+	trace_printk(DM_NAME ": " DM_MSG_PREFIX ": " f "\n", ## arg)
+
 #ifdef CONFIG_DM_DEBUG
 #  define DMDEBUG(f, arg...) \
 	printk(KERN_DEBUG DM_NAME ": " DM_MSG_PREFIX " DEBUG: " f "\n", ## arg)