All of lore.kernel.org
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Ingo Molnar <mingo@elte.hu>,
	Andrew Morton <akpm@linux-foundation.org>,
	Nick Piggin <nickpiggin@yahoo.com.au>,
	Rik van Riel <riel@redhat.com>,
	Lee Schermerhorn <Lee.Schermerhorn@hp.com>
Subject: [RFC][PATCH 02/10] mm: Remove likely() from mapping_unevictable()
Date: Mon, 06 Dec 2010 20:58:36 -0500	[thread overview]
Message-ID: <20101207021328.569328536@goodmis.org> (raw)
In-Reply-To: 20101207015834.196176991@goodmis.org

[-- Attachment #1: 0002-mm-Remove-likely-from-mapping_unevictable.patch --]
[-- Type: text/plain, Size: 3039 bytes --]

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.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Nick Piggin <nickpiggin@yahoo.com.au>
Cc: Rik van Riel <riel@redhat.com>
Cc: Lee Schermerhorn <Lee.Schermerhorn@hp.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 include/linux/pagemap.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 2d1ffe3..9c66e99 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -48,7 +48,7 @@ static inline void mapping_clear_unevictable(struct address_space *mapping)
 
 static inline int mapping_unevictable(struct address_space *mapping)
 {
-	if (likely(mapping))
+	if (mapping)
 		return test_bit(AS_UNEVICTABLE, &mapping->flags);
 	return !!mapping;
 }
-- 
1.7.2.3



  parent reply	other threads:[~2010-12-07  2:13 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 ` Steven Rostedt [this message]
2010-12-07  2:22   ` [RFC][PATCH 02/10] mm: Remove likely() from mapping_unevictable() Steven Rostedt
2010-12-07  7:02     ` Nick Piggin
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=20101207021328.569328536@goodmis.org \
    --to=rostedt@goodmis.org \
    --cc=Lee.Schermerhorn@hp.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=nickpiggin@yahoo.com.au \
    --cc=riel@redhat.com \
    /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.