All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Kirill A. Shutemov" <kirill@shutemov.name>
To: Vladimir Davydov <vdavydov@parallels.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	Rik van Riel <riel@redhat.com>, Hugh Dickins <hughd@google.com>
Subject: Re: [PATCH v2] rmap: fix theoretical race between do_wp_page and shrink_active_list
Date: Tue, 12 May 2015 13:48:24 +0300	[thread overview]
Message-ID: <20150512104824.GC18365@node.dhcp.inet.fi> (raw)
In-Reply-To: <1431425919-28057-1-git-send-email-vdavydov@parallels.com>

On Tue, May 12, 2015 at 01:18:39PM +0300, Vladimir Davydov wrote:
> As noted by Paul the compiler is free to store a temporary result in a
> variable on stack, heap or global unless it is explicitly marked as
> volatile, see:
> 
>   http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4455.html#sample-optimizations
> 
> This can result in a race between do_wp_page() and shrink_active_list()
> as follows.
> 
> In do_wp_page() we can call page_move_anon_rmap(), which sets
> page->mapping as follows:
> 
>   anon_vma = (void *) anon_vma + PAGE_MAPPING_ANON;
>   page->mapping = (struct address_space *) anon_vma;
> 
> The page in question may be on an LRU list, because nowhere in
> do_wp_page() we remove it from the list, neither do we take any LRU
> related locks. Although the page is locked, shrink_active_list() can
> still call page_referenced() on it concurrently, because the latter does
> not require an anonymous page to be locked:
> 
>   CPU0                          CPU1
>   ----                          ----
>   do_wp_page                    shrink_active_list
>    lock_page                     page_referenced
>                                   PageAnon->yes, so skip trylock_page
>    page_move_anon_rmap
>     page->mapping = anon_vma
>                                   rmap_walk
>                                    PageAnon->no
>                                    rmap_walk_file
>                                     BUG
>     page->mapping += PAGE_MAPPING_ANON
> 
> This patch fixes this race by explicitly forbidding the compiler to
> split page->mapping store in page_move_anon_rmap() with the aid of
> WRITE_ONCE.
> 
> Signed-off-by: Vladimir Davydov <vdavydov@parallels.com>
> Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> Cc: "Kirill A. Shutemov" <kirill@shutemov.name>
> Cc: Rik van Riel <riel@redhat.com>
> Cc: Hugh Dickins <hughd@google.com>

Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

-- 
 Kirill A. Shutemov

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

WARNING: multiple messages have this Message-ID (diff)
From: "Kirill A. Shutemov" <kirill@shutemov.name>
To: Vladimir Davydov <vdavydov@parallels.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	"Paul E. McKenney" <paulmck@linux.vnet.ibm.com>,
	Rik van Riel <riel@redhat.com>, Hugh Dickins <hughd@google.com>
Subject: Re: [PATCH v2] rmap: fix theoretical race between do_wp_page and shrink_active_list
Date: Tue, 12 May 2015 13:48:24 +0300	[thread overview]
Message-ID: <20150512104824.GC18365@node.dhcp.inet.fi> (raw)
In-Reply-To: <1431425919-28057-1-git-send-email-vdavydov@parallels.com>

On Tue, May 12, 2015 at 01:18:39PM +0300, Vladimir Davydov wrote:
> As noted by Paul the compiler is free to store a temporary result in a
> variable on stack, heap or global unless it is explicitly marked as
> volatile, see:
> 
>   http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4455.html#sample-optimizations
> 
> This can result in a race between do_wp_page() and shrink_active_list()
> as follows.
> 
> In do_wp_page() we can call page_move_anon_rmap(), which sets
> page->mapping as follows:
> 
>   anon_vma = (void *) anon_vma + PAGE_MAPPING_ANON;
>   page->mapping = (struct address_space *) anon_vma;
> 
> The page in question may be on an LRU list, because nowhere in
> do_wp_page() we remove it from the list, neither do we take any LRU
> related locks. Although the page is locked, shrink_active_list() can
> still call page_referenced() on it concurrently, because the latter does
> not require an anonymous page to be locked:
> 
>   CPU0                          CPU1
>   ----                          ----
>   do_wp_page                    shrink_active_list
>    lock_page                     page_referenced
>                                   PageAnon->yes, so skip trylock_page
>    page_move_anon_rmap
>     page->mapping = anon_vma
>                                   rmap_walk
>                                    PageAnon->no
>                                    rmap_walk_file
>                                     BUG
>     page->mapping += PAGE_MAPPING_ANON
> 
> This patch fixes this race by explicitly forbidding the compiler to
> split page->mapping store in page_move_anon_rmap() with the aid of
> WRITE_ONCE.
> 
> Signed-off-by: Vladimir Davydov <vdavydov@parallels.com>
> Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
> Cc: "Kirill A. Shutemov" <kirill@shutemov.name>
> Cc: Rik van Riel <riel@redhat.com>
> Cc: Hugh Dickins <hughd@google.com>

Acked-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>

-- 
 Kirill A. Shutemov

  reply	other threads:[~2015-05-12 10:48 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-12 10:18 [PATCH v2] rmap: fix theoretical race between do_wp_page and shrink_active_list Vladimir Davydov
2015-05-12 10:18 ` Vladimir Davydov
2015-05-12 10:48 ` Kirill A. Shutemov [this message]
2015-05-12 10:48   ` Kirill A. Shutemov
2015-05-12 22:28 ` Andrew Morton
2015-05-12 22:28   ` Andrew Morton
2015-05-13  1:21   ` Minchan Kim
2015-05-13  1:21     ` Minchan Kim
2015-05-13  8:08   ` Vladimir Davydov
2015-05-13  8:08     ` Vladimir Davydov
2015-05-13  1:14 ` Minchan Kim
2015-05-13  1:14   ` Minchan Kim
  -- strict thread matches above, loose matches on Subject: below --
2015-05-13  1:43 Minchan Kim
2015-05-13  1:43 ` Minchan Kim
2015-05-13  2:04 ` Rik van Riel
2015-05-13  2:04   ` Rik van Riel
2015-05-13  3:00 Minchan Kim
2015-05-13  3:00 ` Minchan Kim

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=20150512104824.GC18365@node.dhcp.inet.fi \
    --to=kirill@shutemov.name \
    --cc=akpm@linux-foundation.org \
    --cc=hughd@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=riel@redhat.com \
    --cc=vdavydov@parallels.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.