linux-kernel - Re: [PATCH 0/2 v3] tracing: Tracing event profiling updates

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20090919073400.GE15292@elte.hu>
Date:	Sat, 19 Sep 2009 09:34:00 +0200
From:	Ingo Molnar <mingo@...e.hu>
To:	Frederic Weisbecker <fweisbec@...il.com>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Steven Rostedt <rostedt@...dmis.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Li Zefan <lizf@...fujitsu.com>,
	Jason Baron <jbaron@...hat.com>,
	Masami Hiramatsu <mhiramat@...hat.com>
Subject: Re: [PATCH 0/2 v3] tracing: Tracing event profiling updates


* Frederic Weisbecker <fweisbec@...il.com> wrote:

> 
> Ingo,
> 
> Hopefully this is my last attempt.
> This new iteration fixes the syscalls events to correctly handle
> the buffer. In the previous version, they did not care about interrupts.
> 
> I only resend the second patch as only this one has changed since the v2.
> 
> The new branch is in:
> git://git.kernel.org/pub/scm/linux/kernel/git/frederic/random-tracing.git
> 	tracing/core-v3
> 
> Thanks,
> 	Frederic.
> 
> Frederic Weisbecker (2):
>       tracing: Factorize the events profile accounting
>       tracing: Allocate the ftrace event profile buffer dynamically
> 
>  include/linux/ftrace_event.h       |   10 +++-
>  include/linux/syscalls.h           |   24 +++-----
>  include/trace/ftrace.h             |  111 ++++++++++++++++++++---------------
>  kernel/trace/trace_event_profile.c |   79 +++++++++++++++++++++++++-
>  kernel/trace/trace_syscalls.c      |   97 +++++++++++++++++++++++++------
>  5 files changed, 234 insertions(+), 87 deletions(-)

Hm, the naming is quite confusing here i think:

  -132,8 +133,12 @@ struct ftrace_event_call {
         atomic_t                profile_count;
         int                     (*profile_enable)(void);
         void                    (*profile_disable)(void);
 +       char                    *profile_buf;
 +       char                    *profile_buf_nmi;

These are generic events, not just 'profiling' histograms.

Generic events can have _many_ output modi:

 - SVGs                   (perf timeline)
 - histograms             (perf report)
 - traces                 (perf trace)
 - summaries / maximums   (perf sched lat)
 - maps                   (perf sched map)
 - graphs                 (perf report --call-graph)

So it's quite a misnomer to talk just about profiling here. This is an 
event record buffer.

Also, what is the currently maximum possible size of ->profile_buf? The 
max size of an event record? The new codepath looks a bit heavy with 
rcu-lock/unlock and other bits put inbetween - and this is now in the 
event sending critical path. Cannot we do a permanent buffer that needs 
no extra locking/reference protection?

Is the whole thing even justified? I mean, we keep the size of records 
low anyway. It's a _lot_ easier to handle on-stack records, they are the 
ideal (and very fast) dynamic allocator which is NMI and IRQ safe, etc.

	Ingo
--
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/