Messages in this thread | | | Date | Wed, 26 Feb 2014 20:38:53 -0500 | From | Steven Rostedt <> | Subject | Re: [RFA][PATCH 0/4] tracing: Request for acks on fixing tracepoint code |
| |
On Wed, 26 Feb 2014 21:36:18 +0000 (UTC) Mathieu Desnoyers <mathieu.desnoyers@efficios.com> wrote:
> ----- Original Message ----- > > From: "Steven Rostedt" <rostedt@goodmis.org> > > To: linux-kernel@vger.kernel.org > > Cc: "Ingo Molnar" <mingo@kernel.org>, "Andrew Morton" <akpm@linux-foundation.org>, "Peter Zijlstra" > > <peterz@infradead.org>, "Frederic Weisbecker" <fweisbec@gmail.com>, "Mathieu Desnoyers" > > <mathieu.desnoyers@efficios.com> > > Sent: Wednesday, February 26, 2014 2:01:40 PM > > Subject: [RFA][PATCH 0/4] tracing: Request for acks on fixing tracepoint code > > > > [ Request for Acks ] > > > > Due to module tainting, we have tracepoints that silently do not work. > > That will be solved another way. But the trace event infrastructure should > > not be created for tainted modules. That is, the debugfs files should > > not exist for them. > > > > By moving the tracepoint module taint test into tracepoint.h, we can > > reuse that same test when creating the module tracepoint events. > > > > Note, I had to remove the tracepoint.h include from module.h as there > > was nothing in module.h that required tracepoint.h, but this broke > > a couple of event files (migrate.h and writeback.h) because they did > > not include tracepoint.h, and were just lucky that it was included > > by module.h. > > When designing tracepoint.h, a lot of care went into making sure it did > not have needless dependency on other headers, since this header is > expected to be included into many other files and headers, thus posing > a clear risk of becoming yet another root of an include dependency hell.
Well, module.h is included in many more.
> > While I agree on adding the API you propose, why made it a static inline ? > This adds this dependency from tracepoint.h on module.h. Instead, we could > just declare a symbol, and implement a tracepoint_module_has_bad_taint() > within kernel/tracepoint.c. It should not be a fast path anyway, so I don't > see the point it making it a static inline. > > I also recommend sticking to the tracepoint_*() API (rather than trace_*).
Well, as this is now not just for tracepoints, but also used by the trace_events, and because the name is already too big (but descriptive), I rather not change it.
But as a compromise, I can move it to ftrace_event.h instead.
-- Steve
| |