linux-kernel - Re: [PATCH 15/17] mm: Do not use unnecessary atomic operations when adding pages to the LRU

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Tue, 06 May 2014 17:30:53 +0200
From:	Vlastimil Babka <vbabka@...e.cz>
To:	Mel Gorman <mgorman@...e.de>, Linux-MM <linux-mm@...ck.org>,
	Linux-FSDevel <linux-fsdevel@...r.kernel.org>
CC:	Johannes Weiner <hannes@...xchg.org>, Jan Kara <jack@...e.cz>,
	Michal Hocko <mhocko@...e.cz>, Hugh Dickins <hughd@...gle.com>,
	Linux Kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 15/17] mm: Do not use unnecessary atomic operations when
 adding pages to the LRU

On 05/01/2014 10:44 AM, Mel Gorman wrote:
> When adding pages to the LRU we clear the active bit unconditionally. As the
> page could be reachable from other paths we cannot use unlocked operations
> without risk of corruption such as a parallel mark_page_accessed. This
> patch test if is necessary to clear the atomic flag before using an atomic

                                           active

> operation. In the unlikely even this races with mark_page_accesssed the
> consequences are simply that the page may be promoted to the active list
> that might have been left on the inactive list before the patch. This is
> a marginal consequence.

Well if this is racy, then even before the patch, mark_page_accessed 
might have come right after ClearPageActive(page) anyway? Or is the 
changelog saying that this change only extended the race window that 
already existed? If yes it could be more explicit, as now it might sound 
as if the race was introduced.

> Signed-off-by: Mel Gorman <mgorman@...e.de>
> ---
>   include/linux/swap.h | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/swap.h b/include/linux/swap.h
> index da8a250..395dcab 100644
> --- a/include/linux/swap.h
> +++ b/include/linux/swap.h
> @@ -329,13 +329,15 @@ extern void add_page_to_unevictable_list(struct page *page);
>    */
>   static inline void lru_cache_add_anon(struct page *page)
>   {
> -	ClearPageActive(page);
> +	if (PageActive(page))
> +		ClearPageActive(page);
>   	__lru_cache_add(page);
>   }
>
>   static inline void lru_cache_add_file(struct page *page)
>   {
> -	ClearPageActive(page);
> +	if (PageActive(page))
> +		ClearPageActive(page);
>   	__lru_cache_add(page);
>   }
>
>

--
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/