[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <m1ljuwt5dl.fsf@frodo.ebiederm.org>
Date: Thu, 04 Dec 2008 05:52:06 -0800
From: ebiederm@...ssion.com (Eric W. Biederman)
To: Steven Rostedt <rostedt@...dmis.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...e.hu>,
Frederic Weisbecker <fweisbec@...il.com>,
Peter Zijlstra <peterz@...radead.org>,
Dave Hansen <dave@...ux.vnet.ibm.com>,
containers@...ts.osdl.org,
Sukadev Bhattiprolu <sukadev@...ux.vnet.ibm.com>,
"Serge E. Hallyn" <serue@...ibm.com>,
Steven Rostedt <srostedt@...hat.com>
Subject: Re: [PATCH 3/3] ftrace: add ability to only trace swapper tasks
Steven Rostedt <rostedt@...dmis.org> writes:
> On Thu, 4 Dec 2008, Andrew Morton wrote:
>
>> On Thu, 04 Dec 2008 00:26:41 -0500 Steven Rostedt <rostedt@...dmis.org> wrote:
>>
>> > From: Steven Rostedt <srostedt@...hat.com>
>> >
>> > Impact: New feature
>> >
>> > This patch lets the swapper tasks of all CPUS be filtered by the
>> > set_ftrace_pid file.
>>
>> "filtered" is one of those nasty words. It is unclear whether the
>> filteree is included or excluded.
>>
>> > If '0' is echoed into this file, then all the idle tasks (aka swapper)
>> > is flagged to be traced. This affects all CPU idle tasks.
>> >
>>
>> s/is/are/
>
> My English is much better before midnight.
> But warning, it is worse before my first coffee. ;-)
>
>>
>> > Signed-off-by: Steven Rostedt <srostedt@...hat.com>
>>
>> What does this patch actually do? Is swapper currently excluded from
>> tracing for undisclosed reasons and this patch permits it to be traced?
>> If so, why was swapper thus excluded? Or am I totally off track?
>
> Yes, it is excluded. For two reasons:
>
> 1) you can not get to the swapper task using struct pid
> 2) the swapper task is not part of the task link list. Well, the first
> swapper task may be, but not the ones for other CPUS.
>
> In fork.c:
>
> if (likely(p->pid)) {
> [...]
> if (thread_group_leader(p)) {
> [...]
> list_add_tail_rcu(&p->tasks, &init_task.tasks);
>
>
>
>> > ---
>> > kernel/trace/ftrace.c | 74 +++++++++++++++++++++++++++++++++++++++++-------
>> > 1 files changed, 63 insertions(+), 11 deletions(-)
>> >
>> > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
>> > index 10b1d7c..eb57dc1 100644
>> > --- a/kernel/trace/ftrace.c
>> > +++ b/kernel/trace/ftrace.c
>> > @@ -49,6 +49,7 @@ static int last_ftrace_enabled;
>> >
>> > /* set when tracing only a pid */
>> > struct pid *ftrace_pid_trace;
>> > +static struct pid * const ftrace_swapper_pid = (struct pid *)1;
>>
>> eh?
>
> The swapper task has no pid structure
It does have a pid structure it just isn't hashed.
>, if we want to trace it, we
> need to find it outside the pid code. But other parts of ftrace use the
> ftrace_pid_trace to see if it it should only trace the tasks that have
> the trace bit set. We have:
>
> if (ftrace_pid_trace)
> /* only trace this task if it is marked to trace */
>
> But, I get your point. That line deserves a comment ;-)
>> What locking does this traversal need, and does this function have it?
>
> No idea, I only did what Eric suggested.
Sorry I'm tired going fast and was just sketching the highlights.
> And people wonder why we still stick to "task->pid"
Hey thanks for trying to figure it out.
Eric
--
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