2009-03-25 05:20:36

by Steven Rostedt

[permalink] [raw]
Subject: [PATCH 0/5] [PATCH][GIT PULL] remove unnecessary (un)likelys


Ingo,

I guess this can go through you. This is a start of clean ups to
get rid of (un)likelys that are at least 50% incorrect. This series
has some that are 100% incorrect.

The branch profiler used is from 2.6.29 which does not have the
fixed header that is in tip. This branch is also based off of
2.6.29 and not tip.

Perhaps, since some of these cases are 100% wrong they can simply
be reversed. I'm choosing to just remove the annotation, and then
later I'll be using the full branch profiler to look for candidates
for adding (un)likelys. That way each added annotation can be
scrutinized individually.

Please pull the latest annotate-branch/cleanups tree, which can be found at:

git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
annotate-branch/cleanups


Steven Rostedt (5):
ptrace: remove incorrect unlikelys
mm: remove unlikly NULL from kfree
sched: remove unlikely in pre_schedule_rt
sched: remove unlikelys from sched_move_task
mm: remove unlikelys for unlock in rmap.c

----
arch/x86/kernel/ptrace.c | 4 ++--
kernel/sched.c | 4 ++--
kernel/sched_rt.c | 2 +-
mm/rmap.c | 8 ++++----
mm/slab.c | 2 +-
mm/slob.c | 2 +-
mm/slub.c | 2 +-
7 files changed, 12 insertions(+), 12 deletions(-)
--


2009-03-25 07:20:26

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 0/5] [PATCH][GIT PULL] remove unnecessary (un)likelys


* Steven Rostedt <[email protected]> wrote:

> Ingo,
>
> I guess this can go through you. This is a start of clean ups to
> get rid of (un)likelys that are at least 50% incorrect. This
> series has some that are 100% incorrect.

Nice!

I'll wait for the acks from affected (and Cc:-ed) maintainers though
- and they might decide to carry the patches in their trees straight
away (or disagree with the change) so a rebase is in the cards as
well.

Ingo

2009-03-25 07:25:45

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 0/5] [PATCH][GIT PULL] remove unnecessary (un)likelys


* Steven Rostedt <[email protected]> wrote:

> ----
> arch/x86/kernel/ptrace.c | 4 ++--
> kernel/sched.c | 4 ++--
> kernel/sched_rt.c | 2 +-
> mm/rmap.c | 8 ++++----
> mm/slab.c | 2 +-
> mm/slob.c | 2 +-
> mm/slub.c | 2 +-
> 7 files changed, 12 insertions(+), 12 deletions(-)

I commented on the ptrace one - i think we should take that one out
or at least wait for more info.

The MM ones look correct. Maybe your kfree observation will be
debated - but i think it's probably right that we should not special
annotate kfree(NULL) anymore in any direction. But it's ultimately
up to the MM folks. (I've Cc:-ed Pekka too)

The rmap and the scheduler ones definitely look correct. When you
rebase, please include the profiler output to 3/5 too.

Thanks,

Ingo

2009-03-25 13:44:00

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH 0/5] [PATCH][GIT PULL] remove unnecessary (un)likelys


On Wed, 25 Mar 2009, Ingo Molnar wrote:

>
> * Steven Rostedt <[email protected]> wrote:
>
> > ----
> > arch/x86/kernel/ptrace.c | 4 ++--
> > kernel/sched.c | 4 ++--
> > kernel/sched_rt.c | 2 +-
> > mm/rmap.c | 8 ++++----
> > mm/slab.c | 2 +-
> > mm/slob.c | 2 +-
> > mm/slub.c | 2 +-
> > 7 files changed, 12 insertions(+), 12 deletions(-)
>
> I commented on the ptrace one - i think we should take that one out
> or at least wait for more info.
>
> The MM ones look correct. Maybe your kfree observation will be
> debated - but i think it's probably right that we should not special
> annotate kfree(NULL) anymore in any direction. But it's ultimately
> up to the MM folks. (I've Cc:-ed Pekka too)
>
> The rmap and the scheduler ones definitely look correct. When you
> rebase, please include the profiler output to 3/5 too.

I'll rebase it to only include the non controversial changes.

Thanks,

-- Steve

2009-04-27 16:41:24

by Daniel Walker

[permalink] [raw]
Subject: Re: [PATCH 0/5] [PATCH][GIT PULL] remove unnecessary (un)likelys

On Wed, 2009-03-25 at 01:19 -0400, Steven Rostedt wrote:
> Ingo,
>
> I guess this can go through you. This is a start of clean ups to
> get rid of (un)likelys that are at least 50% incorrect. This series
> has some that are 100% incorrect.
>
> The branch profiler used is from 2.6.29 which does not have the
> fixed header that is in tip. This branch is also based off of
> 2.6.29 and not tip.
>
> Perhaps, since some of these cases are 100% wrong they can simply
> be reversed. I'm choosing to just remove the annotation, and then
> later I'll be using the full branch profiler to look for candidates
> for adding (un)likelys. That way each added annotation can be
> scrutinized individually.

What kind of methodology are you using to determine which to remove? It
looks like some you review the code, and other you just remove based on
it being %50 wrong or more.. I found workloads are especially important.
Where finding one that is %50 or %100 wrong does necessarily mean
anything..


Daniel