On Wed, 5 Jul 2017 17:11:47 -0700
Joel Fernandes <joe...@google.com> wrote:
> Currently we stop recording task information if any of them error out or are
> not being recorded. Further if switching to/from the idle thread, we bail out
> early on. Fix these issues.
Can you break this up into three patches. And yeah, I'll send these
after my initial pull request to Linus.
>
> Cc: kernel-t...@android.com
> Cc: Steven Rostedt <rost...@goodmis.org>
> Cc: Ingo Molnar <mi...@redhat.com>
> Reported-by: Michael Sartain <mikes...@gmail.com>
> Signed-off-by: Joel Fernandes <joe...@google.com>
> ---
> Hi Steve,
> This fix is based on ftrace/core branch at:
> git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
> Could you pick it up for -rc cycle if it looks good to you? Thanks.
>
> kernel/trace/trace.c | 34 +++++++++++++++++++++++-----------
> 1 file changed, 23 insertions(+), 11 deletions(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index b66a1c8805f3..27b981a46389 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -1918,7 +1918,11 @@ static int trace_save_cmdline(struct task_struct *tsk)
> {
> unsigned pid, idx;
>
> - if (!tsk->pid || unlikely(tsk->pid > PID_MAX_DEFAULT))
> + /* treat recording of idle task as a success */
> + if (!tsk->pid)
> + return 1;
> +
> + if (unlikely(tsk->pid > PID_MAX_DEFAULT))
> return 0;
Make this a separate patch. This should go to stable.
>
> /*
> @@ -2004,7 +2008,11 @@ int trace_find_tgid(int pid)
>
> static int trace_save_tgid(struct task_struct *tsk)
> {
> - if (unlikely(!tgid_map || !tsk->pid || tsk->pid > PID_MAX_DEFAULT))
> + /* treat recording of idle task as a success */
> + if (!tsk->pid)
> + return 1;
> +
> + if (unlikely(!tgid_map || tsk->pid > PID_MAX_DEFAULT))
> return 0;
Make this a separate patch as it fixes a different issue than what is
below.
>
> tgid_map[tsk->pid] = tsk->tgid;
> @@ -2031,11 +2039,14 @@ static bool tracing_record_taskinfo_skip(int flags)
> */
> void tracing_record_taskinfo(struct task_struct *task, int flags)
> {
> + bool done;
> +
> if (tracing_record_taskinfo_skip(flags))
> return;
> - if ((flags & TRACE_RECORD_CMDLINE) && !trace_save_cmdline(task))
> - return;
> - if ((flags & TRACE_RECORD_TGID) && !trace_save_tgid(task))
> +
> + done = !(flags & TRACE_RECORD_CMDLINE) || trace_save_cmdline(task);
> + done &= !(flags & TRACE_RECORD_TGID) || trace_save_tgid(task);
> + if (!done)
> return;
Should probably add some comments before the return.
>
> __this_cpu_write(trace_taskinfo_save, false);
> @@ -2052,15 +2063,16 @@ void tracing_record_taskinfo(struct task_struct
> *task, int flags)
> void tracing_record_taskinfo_sched_switch(struct task_struct *prev,
> struct task_struct *next, int flags)
> {
> - if (tracing_record_taskinfo_skip(flags))
> - return;
> + bool done;
>
> - if ((flags & TRACE_RECORD_CMDLINE) &&
> - (!trace_save_cmdline(prev) || !trace_save_cmdline(next)))
> + if (tracing_record_taskinfo_skip(flags))
> return;
>
> - if ((flags & TRACE_RECORD_TGID) &&
> - (!trace_save_tgid(prev) || !trace_save_tgid(next)))
> + done = !(flags & TRACE_RECORD_CMDLINE) || trace_save_cmdline(prev);
> + done &= !(flags & TRACE_RECORD_CMDLINE) || trace_save_cmdline(next);
> + done &= !(flags & TRACE_RECORD_TGID) || trace_save_tgid(prev);
> + done &= !(flags & TRACE_RECORD_TGID) || trace_save_tgid(next);
Some comments here too.
Thanks!
-- Steve
> + if (!done)
> return;
>
> __this_cpu_write(trace_taskinfo_save, false);
