Ingo,
The following patches clean up the unlikely/likely tracer. Namely
it consolidates it into a single file called "profile_annotated_branch".
It also adds a new profiler. A true branch profiler that profiles all
if() statements where the conditional is not a constant. It puts
a bit of overhead on the system, but the results seem pretty interesting.
The results are placed in "profile_branch".
Anyway, enjoy ;-)
The following patches are in:
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
branch: tip/devel
Steven Rostedt (4):
trace: remove extra assign in branch check
trace: consolidate unlikely and likely profiler
trace: branch profiling should not print percent without data
trace: profile all if conditionals
----
include/asm-generic/vmlinux.lds.h | 20 +++++++----
include/linux/compiler.h | 64 +++++++++++++++++++++------------
kernel/trace/Kconfig | 19 +++++++++-
kernel/trace/trace_branch.c | 72 +++++++++++++++++++++++-------------
4 files changed, 117 insertions(+), 58 deletions(-)
On Fri, 2008-11-21 at 02:12 -0500, Steven Rostedt wrote:
> Ingo,
>
> The following patches clean up the unlikely/likely tracer. Namely
> it consolidates it into a single file called "profile_annotated_branch".
>
> It also adds a new profiler. A true branch profiler that profiles all
> if() statements where the conditional is not a constant. It puts
> a bit of overhead on the system, but the results seem pretty interesting.
> The results are placed in "profile_branch".
>
I looked at the full version of this, and it looks really slow.. As I
recall the biggest problem with the -mm version was it's cacheline
bouncing (pointed out by Ingo), and yours doesn't _seem_ to fix that. In
fact your version looks a lot worse..
So really between the two if we want mainline likely profiling the -mm
version is a better choice.. The reason that version never went into
mainline is cause neither me or Andrew felt strongly that this was
useful in more than just -mm ..
If you look at the output from the profiling long enough it becomes
clear that it's frequently misleading .. In the short term a certain
branch might be likely, and in the long term it isn't.. So you can't
really blindly start converting the annotation..
I should also mention that I didn't write the -mm version alone, it was
an effort between three people me, Andrew, and Hua Zhong (CC added)..
Daniel
On Sun, 30 Nov 2008, Daniel Walker wrote:
> On Fri, 2008-11-21 at 02:12 -0500, Steven Rostedt wrote:
> > Ingo,
> >
> > The following patches clean up the unlikely/likely tracer. Namely
> > it consolidates it into a single file called "profile_annotated_branch".
> >
> > It also adds a new profiler. A true branch profiler that profiles all
> > if() statements where the conditional is not a constant. It puts
> > a bit of overhead on the system, but the results seem pretty interesting.
> > The results are placed in "profile_branch".
> >
>
> I looked at the full version of this, and it looks really slow.. As I
> recall the biggest problem with the -mm version was it's cacheline
> bouncing (pointed out by Ingo), and yours doesn't _seem_ to fix that. In
> fact your version looks a lot worse..
I could probably do more to fix the cache line bouncing, but that is not
the number one concern rigth now. It is already a big overhead, both
memory and performance. That will be something I may address later.
>
> So really between the two if we want mainline likely profiling the -mm
> version is a better choice.. The reason that version never went into
> mainline is cause neither me or Andrew felt strongly that this was
> useful in more than just -mm ..
This once is also packaged with ftrace, so it makes it nice and tidy ;-)
I'm sure more people will be interested in such a thing outside of -mm.
I doubt that it will ever be turned on in a production kernel.
>
> If you look at the output from the profiling long enough it becomes
> clear that it's frequently misleading .. In the short term a certain
> branch might be likely, and in the long term it isn't.. So you can't
> really blindly start converting the annotation..
I'd go with both. If it should not be likely in both the short and long
term than it should not be likely (same goes with unlikely). I think this
will serve more in the case of removing unlikely's than to add them.
>
> I should also mention that I didn't write the -mm version alone, it was
> an effort between three people me, Andrew, and Hua Zhong (CC added)..
OK, that is good to know. There was just a couple of things that I took
from your patch set and I stated them. I wrote the first version without
even knowing about you patch, and it was Andrew that pointed it out to me.
The two things that I took was the use of the builtin_constant_p and to
use __FILE__ instead of IP.
Thanks,
-- Steve