linux-kernel - Re: [PATCH 3/3] ftrace: add ability to only trace swapper tasks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

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/