lkml.org 
[lkml]   [2015]   [Jan]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] tracing: Allow raw_syscall tracepoints to work from boot
On Wed, 14 Jan 2015 09:35:17 +1100
Michael Ellerman <mpe@ellerman.id.au> wrote:

> In commit 5f893b2639b2 "tracing: Move enabling tracepoints to just after
> rcu_init()", tracing was enabled earlier in boot.
>
> This broke tracing of the raw_syscall tracepoints from boot using the
> trace_event kernel parameter.
>
> When the registration function for the raw_syscall tracepoints runs, it
> iterates over all tasks setting TIF_SYSCALL_TRACEPOINT. However now that
> this happens earlier in boot, the loop has no effect because there are
> no tasks in existence other than init_task, which is skipped by the
> for_each_process_thread() macro.
>
> We can fix it by explicitly setting TIF_SYSCALL_TRACEPOINT for the
> init_task. That way when pid 1 is cloned from init_task it will inherit
> TIF_SYSCALL_TRACEPOINT.
>
> Fixes: 5f893b2639b2 ("tracing: Move enabling tracepoints to just after rcu_init()")

I don't like setting the swap task flag for syscall tracing, as nothing
will unset it.


> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
> kernel/tracepoint.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
>
> It feels a bit naughty to be whacking init_task like this, but it also
> seems like the right fix?

No, I tried the following instead.

>
> Should we also clear it in the unregfunc? I can't see how that would
> ever be needed in practice?

It just seems hacky to set swapper in the first place.

Try my patch and let me know if it works for you?

-- Steve

From aae3b6410d536d65204cce7654887095dc41e1cf Mon Sep 17 00:00:00 2001
From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>
Date: Wed, 14 Jan 2015 12:53:45 -0500
Subject: [PATCH] tracing: Fix enabling of syscall events on the command line

Commit 5f893b2639b2 "tracing: Move enabling tracepoints to just after
rcu_init()" broke the enabling of system call events from the command
line. The reason was that the enabling of command line trace events
was moved before PID 1 started, and the syscall tracepoints require
that all tasks have the TIF_SYSCALL_TRACEPOINT flag set. But the
swapper task (pid 0) is not part of that. Since the swapper task is the
only task that is running at this early in boot, no task gets the
flag set, and the tracepoint never gets reached.

Instead of setting the swapper task flag (there should be no reason to
do that), re-enabled trace events again after the init thread (PID 1)
has been started. It requires disabling all command line events and
re-enabling them, as just enabling them again will not reset the logic
to set the TIF_SYSCALL_TRACEPOINT flag, as the syscall tracepoint will
be fooled into thinking that it was already set, and wont try setting
it again. For this reason, we must first disable it and re-enable it.

Link: http://lkml.kernel.org/r/1421188517-18312-1-git-send-email-mpe@ellerman.id.au

Reported-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
kernel/trace/trace_events.c | 69 ++++++++++++++++++++++++++++++++++++---------
1 file changed, 55 insertions(+), 14 deletions(-)

diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 366a78a3e61e..b03a0ea77b99 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -2429,12 +2429,39 @@ static __init int event_trace_memsetup(void)
return 0;
}

+static __init void
+early_enable_events(struct trace_array *tr, bool disable_first)
+{
+ char *buf = bootup_event_buf;
+ char *token;
+ int ret;
+
+ while (true) {
+ token = strsep(&buf, ",");
+
+ if (!token)
+ break;
+ if (!*token)
+ continue;
+
+ /* Restarting syscalls requires that we stop them first */
+ if (disable_first)
+ ftrace_set_clr_event(tr, token, 0);
+
+ ret = ftrace_set_clr_event(tr, token, 1);
+ if (ret)
+ pr_warn("Failed to enable trace event: %s\n", token);
+
+ /* Put back the comma to allow this to be called again */
+ if (buf)
+ *(buf - 1) = ',';
+ }
+}
+
static __init int event_trace_enable(void)
{
struct trace_array *tr = top_trace_array();
struct ftrace_event_call **iter, *call;
- char *buf = bootup_event_buf;
- char *token;
int ret;

if (!tr)
@@ -2456,18 +2483,7 @@ static __init int event_trace_enable(void)
*/
__trace_early_add_events(tr);

- while (true) {
- token = strsep(&buf, ",");
-
- if (!token)
- break;
- if (!*token)
- continue;
-
- ret = ftrace_set_clr_event(tr, token, 1);
- if (ret)
- pr_warn("Failed to enable trace event: %s\n", token);
- }
+ early_enable_events(tr, false);

trace_printk_start_comm();

@@ -2478,6 +2494,31 @@ static __init int event_trace_enable(void)
return 0;
}

+/*
+ * event_trace_enable() is called from trace_event_init() first to
+ * initialize events and perhaps start any events that are on the
+ * command line. Unfortunately, there are some events that will not
+ * start this early, like the system call tracepoints that need
+ * to set the TIF_SYSCALL_TRACEPOINT flag of pid 1. But event_trace_enable()
+ * is called before pid 1 starts, and this flag is never set, making
+ * the syscall tracepoint never get reached, but the event is enabled
+ * regardless (and not doing anything).
+ */
+static __init int event_trace_enable_again(void)
+{
+ struct trace_array *tr;
+
+ tr = top_trace_array();
+ if (!tr)
+ return -ENODEV;
+
+ early_enable_events(tr, true);
+
+ return 0;
+}
+
+early_initcall(event_trace_enable_again);
+
static __init int event_trace_init(void)
{
struct trace_array *tr;
--
1.8.1.4


\
 
 \ /
  Last update: 2015-01-14 19:21    [W:0.073 / U:0.116 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site