Re: [RFC][PATCH 02/10] mm: Remove likely() from mapping_unevictable()

Steven Rostedt
2010-12-07T02:22:16Z

[ 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.

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;
}