linux-kernel - Re: [RFC][PATCH 02/10] mm: Remove likely() from mapping

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 10 Dec 2010 16:00:16 +0900 (JST)
From:	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>
To:	Steven Rostedt <rostedt@...dmis.org>
Cc:	kosaki.motohiro@...fujitsu.com, linux-kernel@...r.kernel.org,
	Ingo Molnar <mingo@...e.hu>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Nick Piggin <npiggin@...nel.dk>,
	Rik van Riel <riel@...hat.com>,
	Lee Schermerhorn <Lee.Schermerhorn@...com>
Subject: Re: [RFC][PATCH 02/10] mm: Remove likely() from mapping_unevictable()

> [ Resending to Nick's real email ]
> 
> 
> From: Steven Rostedt <srostedt@...hat.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@...ux-foundation.org>
> Cc: Nick Piggin <nickpiggin@...oo.com.au>
> Cc: Rik van Riel <riel@...hat.com>
> Cc: Lee Schermerhorn <Lee.Schermerhorn@...com>
> Signed-off-by: Steven Rostedt <rostedt@...dmis.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;
>  }

I think you are right.
	Reviewed-by: KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/