lkml.org 
[lkml]   [2009]   [Aug]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] tracing/profile: Fix profile_disable vs module_unload

On Thu, 27 Aug 2009, Li Zefan wrote:

> > Peter is correct that he should not need to worry about modules, he
> > doesn't build kernels with them ;-)
> >
> > Here's another patch that moves the module ref count administration to the
> > trace events themselves. This should satisfy both you and Peter.
> >
>
> In fact I had this patch in my mind, but I thought Peter insist
> on fixing it in tracepoint. So I'm fine with this. :)

Tracepoints I consider a more low level API. Using them takes more hand
work and the user needs to know what they are doing and thus, must take
into account modules.

This code is automatic, and a much higher level API. The user shoud not
need to worry about modules, thus the protection belongs here.

If we try to make it so a module can not have register itself, then that
will just complicate the TRACE_EVENT macros even more. And those are
complex enough.

>
> > Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> >
> > diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
> > index 1b1f742..3f7c5dc 100644
> > --- a/include/trace/ftrace.h
> > +++ b/include/trace/ftrace.h
> > @@ -390,6 +390,20 @@ static inline int ftrace_get_offsets_##call( \
> > *
> > */
> >
> > +#ifdef MODULE
> > +# define event_trace_up_ref() \
> > + do { \
> > + if (!try_module_get(THIS_MODULE)) { \
> > + atomic_dec(&event_call->profile_count); \
> > + return -1; \
>
> Should return -1 or -errno like -ENOENT?

Thanks, I was being lazy and really did not know what to have it return.
I'll commit it with a -ENOENT.

-- Steve

>
> > + } \
> > + } 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
> > +
> > #undef TRACE_EVENT
> > #define TRACE_EVENT(call, proto, args, tstruct, assign, print) \
> > \
> > @@ -399,16 +413,20 @@ 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)) { \
> > + event_trace_up_ref(); \
> > ret = register_trace_##call(ftrace_profile_##call); \
> > + } \
> > \
> > return ret; \
> > } \
> > \
> > 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); \
> > + event_trace_down_ref(); \
> > + } \
> > }
> >
> > #include TRACE_INCLUDE(TRACE_INCLUDE_FILE)
> >
> >
>


\
 
 \ /
  Last update: 2009-08-27 04:17    [W:0.178 / U:0.480 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site