[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1252680025.18996.684.camel@gandalf.stny.rr.com>
Date: Fri, 11 Sep 2009 10:40:25 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: Peter Zijlstra <peterz@...radead.org>
Cc: linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...e.hu>,
Andrew Morton <akpm@...ux-foundation.org>,
Frederic Weisbecker <fweisbec@...il.com>,
Li Zefan <lizf@...fujitsu.com>,
Mathieu Desnoyers <compudj@...stal.dyndns.org>
Subject: Re: [PATCH 2/3] tracing/profile: add ref count for registering
profile events
On Fri, 2009-09-11 at 16:09 +0200, Peter Zijlstra wrote:
> On Fri, 2009-09-11 at 16:04 +0200, Peter Zijlstra wrote:
> > On Fri, 2009-09-11 at 09:54 -0400, Steven Rostedt wrote:
> >
> > > +#ifdef MODULE
> > > +# define event_trace_up_ref() \
> > > + do { \
> > > + if (!try_module_get(THIS_MODULE)) { \
> > > + atomic_dec(&event_call->profile_count); \
> > > + return -ENOENT; \
> > > + } \
> > > + } while (0)
> > > +# define event_trace_down_ref() module_put(THIS_MODULE)
> > > +#else
> > > +# define event_trace_up_ref() do { } while (0)
> > > +# define event_trace_down_ref() do { } while (0)
> > > +#endif
> >
> > That's like truely gruesomely ugly.
> >
> > At the very least write it like:
> >
> > int event_trace_up_ref(struct ftrace_event_call *call)
> > {
> > if (!try_module_get(THIS_MODULE)) {
> > atomic_dev(&call->profile_count);
> > return -ENOENT;
> > }
> > return 0;
> > }
>
> Or we can go with Li's original patch, that was less ugly.
Here's another version of the patch (less ugly). I forgot that the
try_module_get should handle code compiled with !CONFIG_MODULE.
This should be less ugly.
-- Steve
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index 308bafd..6d5edd1 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -409,16 +409,23 @@ static int ftrace_profile_enable_##call(struct ftrace_event_call *event_call) \
{ \
int ret = 0; \
\
- if (!atomic_inc_return(&event_call->profile_count)) \
+ if (!atomic_inc_return(&event_call->profile_count)) { \
+ if (!try_module_get(THIS_MODULE)) \
+ goto out_fail; \
ret = register_trace_##call(ftrace_profile_##call); \
+ } \
\
return ret; \
+ out_fail: \
+ atomic_dec(&event_call->profile_count); \
} \
\
static void ftrace_profile_disable_##call(struct ftrace_event_call *event_call)\
{ \
- if (atomic_add_negative(-1, &event_call->profile_count)) \
+ if (atomic_add_negative(-1, &event_call->profile_count)) { \
unregister_trace_##call(ftrace_profile_##call); \
+ module_put(THIS_MODULE) \
+ } \
}
#include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists