From: Tejun Heo <tj@kernel.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: mingo@elte.hu, efault@gmx.de, avi@redhat.com, paulus@samba.org,
acme@redhat.com, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCHSET] sched,perf: unify tracers in sched and move perf on top of TP
Date: Wed, 05 May 2010 11:32:35 +0200 [thread overview]
Message-ID: <4BE13B33.3030709@kernel.org> (raw)
In-Reply-To: <1273050400.1642.229.camel@laptop>
Hello,
On 05/05/2010 11:06 AM, Peter Zijlstra wrote:
>> Yeap, it's not the prettiest thing. I thought about renaming all of
>> them so that they share the same postfix but then again I need a way
>> to tell the script which to enable which is the easiest with
>> specifying postfixes as macro argument. Any better ideas?
>
> Add more NOP functions for the missing bits I guess, but that gets
> cumbersome too.
Hmm... yeah, we'll need NOP functions for every missing hook. Might
as well just spell each one out.
>> Hmm... What overhead?
>
> Well, the perf_{inc,dec}_nr_events() stuff, that's massively painful. If
> that is the price to pay for using tracepoints then I'd rather not.
Ah, okay, you mean the code complexity overhead. Eh well, if you
wanna do it on-demand, there's certain amount you need to pay. As
long as the runtime overhead on hotpath isn't increased, I don't think
it's too bad tho.
> Also, the whole magic dance to keep !CONFIG_TRACEPOINTS working doesn't
> make the code any saner.
Things would have been much prettier if PERF could simply depend on
TRACEPOINTS. More on this later.
> Well, in the current code we get the call overhead, but all perf
> functions bail on !nr_events. We could invert that and have an inline
> function check nr_events and only then do the call.
The thing is that unifying hooks not only saves the extra check now
but also makes future optimizations much easier. ie. Ingo was talking
about run time patching TPs so that disabled ones just become noops.
It's far better to have all the tracing stuff on single hooking
mechanism for optimization efforts like that.
> Right, recent Intel chips seem to do much better at IRQ disable than say
> P4 and IA64, and poking at the PMU MSRs is way more painful, not sure
> how it would balance out for PowerPC (or anything other) though.
As I wrote above, here, the problem is mostly cosmetic. It's just
that kvm hook needs irq on while perf hook can save one irq flipping
if called with irq off. If we call them separately, there's no
problem. If we wanna colocate them, it makes sense to pull perf hook
out of irq disabled section. This one doesn't really make that much
of a difference.
> Well, I already utterly hate that x86 can't build with !
> CONFIG_PERF_EVENTS, also requiring CONFIG_TRACEPOINTS is going the wrong
> way.
>
> I've always explicitly avoided depending on tracepoints, and I'd very
> much like to keep it that way.
I was wondering the other way around - ie. the possibility to make
perf optional and maybe even as a module which depends on TPs, which
would be nicer than the current situation and make the code less
cluttered too.
Thanks.
--
tejun
next prev parent reply other threads:[~2010-05-05 9:33 UTC|newest]
Thread overview: 55+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-05-04 12:38 [RFC PATCHSET] sched,perf: unify tracers in sched and move perf on top of TP Tejun Heo
2010-05-04 12:38 ` [PATCH 01/12] sched: drop @cpu argument from sched_in preempt notifier Tejun Heo
2010-05-04 17:11 ` Peter Zijlstra
2010-05-04 12:38 ` [PATCH 02/12] sched: rename preempt_notifiers to sched_notifiers and refactor implementation Tejun Heo
2010-05-04 12:38 ` [PATCH 03/12] perf: add perf_event_task_migrate() Tejun Heo
2010-05-05 5:08 ` Frederic Weisbecker
2010-05-05 5:16 ` Tejun Heo
2010-05-05 9:11 ` Peter Zijlstra
2010-05-05 9:37 ` Tejun Heo
2010-05-05 9:50 ` Peter Zijlstra
2010-05-05 9:56 ` Tejun Heo
2010-05-04 12:38 ` [PATCH 04/12] perf: add @rq to perf_event_task_sched_out() Tejun Heo
2010-05-04 17:11 ` Peter Zijlstra
2010-05-04 17:22 ` Tejun Heo
2010-05-04 17:42 ` Peter Zijlstra
2010-05-04 18:22 ` Steven Rostedt
2010-05-04 18:26 ` Peter Zijlstra
2010-05-04 18:32 ` Steven Rostedt
2010-05-05 4:48 ` Tejun Heo
2010-05-05 9:58 ` Tejun Heo
2010-05-07 18:41 ` [tip:sched/core] sched: Remove rq argument to the tracepoints tip-bot for Peter Zijlstra
2010-05-04 12:38 ` [PATCH 05/12] perf: move perf_event_task_sched_in() next to fire_sched_notifiers_in() Tejun Heo
2010-05-04 12:38 ` [PATCH 06/12] sched: relocate fire_sched_notifiers_out() and trace_sched_switch() Tejun Heo
2010-05-04 12:38 ` [PATCH 07/12] sched: coalesce event notifiers Tejun Heo
2010-05-04 12:38 ` [PATCH 08/12] sched: add switch_in and tick tracepoints Tejun Heo
2010-05-04 12:38 ` [PATCH 09/12] perf: factor out perf_event_switch_clones() Tejun Heo
2010-05-04 12:38 ` [PATCH 10/12] perf: make nr_events an int and add perf_online_mutex to protect it Tejun Heo
2010-05-04 12:38 ` [PATCH 11/12] perf: prepare to move sched perf functions on top of tracepoints Tejun Heo
2010-05-04 12:38 ` [PATCH 12/12] perf: " Tejun Heo
2010-05-04 17:29 ` [RFC PATCHSET] sched,perf: unify tracers in sched and move perf on top of TP Peter Zijlstra
2010-05-05 5:00 ` Tejun Heo
2010-05-05 9:06 ` Peter Zijlstra
2010-05-05 9:32 ` Tejun Heo [this message]
2010-05-05 9:51 ` Peter Zijlstra
2010-05-05 9:54 ` Tejun Heo
2010-05-05 11:38 ` Peter Zijlstra
2010-05-05 12:28 ` Tejun Heo
2010-05-05 16:55 ` Ingo Molnar
2010-05-05 18:12 ` Peter Zijlstra
2010-05-05 18:16 ` Peter Zijlstra
2010-05-05 18:30 ` Frederic Weisbecker
2010-05-06 6:28 ` Ingo Molnar
2010-05-06 7:11 ` Tejun Heo
2010-05-06 8:27 ` Ingo Molnar
2010-05-06 8:41 ` Tejun Heo
2010-05-06 8:18 ` Avi Kivity
2010-05-06 6:31 ` Ingo Molnar
2010-05-06 7:04 ` Peter Zijlstra
2010-05-06 7:11 ` Ingo Molnar
2010-05-06 7:29 ` Tejun Heo
2010-05-06 7:33 ` Tejun Heo
2010-05-05 12:33 ` Avi Kivity
2010-05-05 13:09 ` Tejun Heo
2010-05-10 5:20 ` Paul Mackerras
2010-05-10 5:48 ` Tejun Heo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4BE13B33.3030709@kernel.org \
--to=tj@kernel.org \
--cc=acme@redhat.com \
--cc=avi@redhat.com \
--cc=efault@gmx.de \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=paulus@samba.org \
--cc=peterz@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.