Date:	Tue, 19 Oct 2010 10:00:47 -0400
From:	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	Koki Sanagi <sanagi.koki@...fujitsu.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...e.hu>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Steven Rostedt <rostedt@...dmis.org>, nhorman@...driver.com,
	scott.a.mcmillan@...el.com, laijs@...fujitsu.com,
	"H. Peter Anvin" <hpa@...or.com>,
	LKML <linux-kernel@...r.kernel.org>, eric.dumazet@...il.com,
	kaneshige.kenji@...fujitsu.com, David Miller <davem@...emloft.net>,
	izumi.taku@...fujitsu.com, kosaki.motohiro@...fujitsu.com,
	Heiko Carstens <heiko.carstens@...ibm.com>,
	"Luck, Tony" <tony.luck@...el.com>
Subject: Re: [PATCH] tracing: Cleanup the convoluted softirq tracepoints

* Thomas Gleixner (tglx@...utronix.de) wrote:
> On Tue, 19 Oct 2010, Mathieu Desnoyers wrote:
> 
> > * Thomas Gleixner (tglx@...utronix.de) wrote:
> > > With the addition of trace_softirq_raise() the softirq tracepoint got
> > > even more convoluted. Why the tracepoints take two pointers to assign
> > > an integer is beyond my comprehension.
> > > 
> > > But adding an extra case which treats the first pointer as an unsigned
> > > long when the second pointer is NULL including the back and forth
> > > type casting is just horrible.
> > > 
> > > Convert the softirq tracepoints to take a single unsigned int argument
> > > for the softirq vector number and fix the call sites.
> > 
> > Well, there was originally a reason for this oddness. The in __do_softirq(),
> > "h - softirq_ve"c computation was not needed outside of the tracepoint handler
> > in the past, but it now seems to be required with the new inlined
> > "kstat_incr_softirqs_this_cpu()".
> 
> Dudes, a vector computation is hardly a performance problem in that
> function and definitely not an excuse for designing such horrible
> interfaces.

In this specific case, I think you are right. But things are not that trivial,
and you know it. We have to consider:

- Extra instruction cache footprint
- Added register pressure
- Added computation overhead of the added substraction
- Frequency of code execution

for all target architectures when we add tracepoints to performance sensitive
code paths. As a general policy, we try to keep these at the lowest possible
level, so that all tracepoints will be compiled into distro kernels without
perceivable _overall_ performance overhead. It's not something that should be
looked at only on a tracepoint-by-tracepoint overhead basis, but rather by
looking at the overall system degradation that adding 300 tracepoints would
cause.

So I agree with you that it's a trade-off between interface cleanness and
performance. When they were introduced, Tracepoint handlers were barely seen as
citizen of the kernel code base, so all that mattered was to keep the
"tracepoints off" case clean and fast. Now that tracepoint handlers seems to be
increasingly accepted as part of the kernel code base, I agree that taking into
account oddness performed in this handler becomes more important. It ends up
being a question of balance between oddness inside the tracepoint handler and
performance overhead in the off-case. The increased acceptance of the tracepoint
code-base has shifted this balance slightly in favor of cleanness.

Thanks,

Mathieu

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
--
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