From: Nick Piggin <npiggin@kernel.dk>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-kernel@vger.kernel.org, Ingo Molnar <mingo@elte.hu>,
Andrew Morton <akpm@linux-foundation.org>,
Nick Piggin <npiggin@kernel.dk>, Rik van Riel <riel@redhat.com>,
Lee Schermerhorn <Lee.Schermerhorn@hp.com>
Subject: Re: [RFC][PATCH 02/10] mm: Remove likely() from mapping_unevictable()
Date: Tue, 7 Dec 2010 18:02:33 +1100 [thread overview]
Message-ID: <20101207070233.GB3307@amd> (raw)
In-Reply-To: <1291688533.16223.119.camel@gandalf.stny.rr.com>
On Mon, Dec 06, 2010 at 09:22:13PM -0500, Steven Rostedt wrote:
> [ Resending to Nick's real email ]
>
>
> From: Steven Rostedt <srostedt@redhat.com>
>
> The mapping_unevictable() has a likely() around the mapping parameter.
> This mapping parameter comes from page_mapping() which has an
> unlikely() that the page will be set as PAGE_MAPPING_ANON, and if
> so, it will return NULL. One would think that this unlikely() means
> that the mapping returned by page_mapping() would not be NULL, but
> where page_mapping() is used just above mapping_unevictable(), that
> unlikely() is incorrect most of the time. This means that the
> "likely(mapping)" in mapping_unevictable() is incorrect most of the
> time.
>
> Running the annotated branch profiler on my main box which runs firefox,
> evolution, xchat and is part of my distcc farm, I had this:
>
> correct incorrect % Function File Line
> ------- --------- - -------- ---- ----
> 12872836 1269443893 98 mapping_unevictable pagemap.h 51
> 35935762 1270265395 97 page_mapping mm.h 659
> 1306198001 143659 0 page_mapping mm.h 657
> 203131478 121586 0 page_mapping mm.h 657
> 5415491 1116 0 page_mapping mm.h 657
> 74899487 1116 0 page_mapping mm.h 657
> 203132845 224 0 page_mapping mm.h 659
> 5415464 27 0 page_mapping mm.h 659
> 13552 0 0 page_mapping mm.h 657
> 13552 0 0 page_mapping mm.h 659
> 242630 0 0 page_mapping mm.h 657
> 242630 0 0 page_mapping mm.h 659
> 74899487 0 0 page_mapping mm.h 659
>
> The page_mapping() is a static inline, which is why it shows up multiple
> times. The mapping_unevictable() is also a static inline but seems to
> be used only once in my setup.
>
> The unlikely in page_mapping() was correct a total of 1909540379 times and
> incorrect 1270533123 times, with a 39% being incorrect. Perhaps this is
> enough to remove the unlikely from page_mapping() as well.
I think so. We don't have good guidelines or empirical numbers on this,
unfortunately, but I think it would be somewhere over 90%, given that it
tends to add jumps to and from out of line icache in the incorrect case.
Acked-by: Nick Piggin <npiggin@kernel.dk>
next prev parent reply other threads:[~2010-12-07 7:02 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-12-07 1:58 [RFC][PATCH 00/10] incorrect unlikely() and likely() cleanups Steven Rostedt
2010-12-07 1:58 ` [RFC][PATCH 01/10] sched: Change rt_task(prev) in pre_schedule_rt to likely Steven Rostedt
2010-12-07 3:25 ` Yong Zhang
2010-12-07 3:32 ` Steven Rostedt
2010-12-07 1:58 ` [RFC][PATCH 02/10] mm: Remove likely() from mapping_unevictable() Steven Rostedt
2010-12-07 2:22 ` Steven Rostedt
2010-12-07 7:02 ` Nick Piggin [this message]
2010-12-07 13:06 ` Steven Rostedt
2010-12-07 16:26 ` Rik van Riel
2010-12-10 7:00 ` KOSAKI Motohiro
2010-12-10 7:06 ` Joe Perches
2010-12-10 8:08 ` Miles Bader
2010-12-11 0:09 ` Steven Rostedt
2010-12-07 1:58 ` [RFC][PATCH 03/10] workqueue: It is likely that WORKER_NOT_RUNNING is true Steven Rostedt
2010-12-07 9:49 ` Tejun Heo
2010-12-07 13:07 ` Steven Rostedt
2010-12-11 0:08 ` Steven Rostedt
2010-12-11 0:09 ` Tejun Heo
2010-12-11 0:12 ` Steven Rostedt
2010-12-07 1:58 ` [RFC][PATCH 04/10] sched: Change pick_next_task_rt from unlikely to likely Steven Rostedt
2010-12-07 2:46 ` Gregory Haskins
2010-12-07 2:59 ` Steven Rostedt
2010-12-11 0:07 ` Steven Rostedt
2010-12-07 1:58 ` [RFC][PATCH 05/10] mm: Remove likely() from grab_cache_page_write_begin() Steven Rostedt
2010-12-07 2:24 ` Steven Rostedt
2010-12-07 6:56 ` Nick Piggin
2010-12-07 1:58 ` [RFC][PATCH 06/10] sched: Remove unlikely() from rt_policy() in sched.c Steven Rostedt
2010-12-07 1:58 ` [RFC][PATCH 07/10] x86: Remove unlikey()s from sched_switch segment tests Steven Rostedt
2010-12-07 1:58 ` [RFC][PATCH 08/10] fs: Remove unlikely() from fput_light() Steven Rostedt
2010-12-07 1:58 ` [RFC][PATCH 09/10] fs: Remove unlikely() from fget_light() Steven Rostedt
2010-12-07 1:58 ` [RFC][PATCH 10/10] sched: Remove unlikely() from ttwu_post_activation Steven Rostedt
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=20101207070233.GB3307@amd \
--to=npiggin@kernel.dk \
--cc=Lee.Schermerhorn@hp.com \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=riel@redhat.com \
--cc=rostedt@goodmis.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.