From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pd0-f175.google.com (mail-pd0-f175.google.com [209.85.192.175]) by kanga.kvack.org (Postfix) with ESMTP id 9C53C6B0038 for ; Tue, 12 May 2015 21:44:01 -0400 (EDT) Received: by pdbqd1 with SMTP id qd1so34022557pdb.2 for ; Tue, 12 May 2015 18:44:01 -0700 (PDT) Received: from mail-pd0-x233.google.com (mail-pd0-x233.google.com. [2607:f8b0:400e:c02::233]) by mx.google.com with ESMTPS id gm10si23531126pbd.98.2015.05.12.18.44.00 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 12 May 2015 18:44:00 -0700 (PDT) Received: by pdea3 with SMTP id a3so33974249pde.3 for ; Tue, 12 May 2015 18:44:00 -0700 (PDT) Date: Wed, 13 May 2015 10:43:52 +0900 From: Minchan Kim Subject: Re: [PATCH v2] rmap: fix theoretical race between do_wp_page and shrink_active_list Message-ID: <20150513014352.GC8267@blaptop> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Sender: owner-linux-mm@kvack.org List-ID: To: Vladimir Davydov Cc: Andrew Morton , linux-mm@kvack.org, linux-kernel@vger.kernel.org, "Paul E. McKenney" , "Kirill A. Shutemov" , Rik van Riel , Hugh Dickins Hi, Rik I'd like to bring up the issue in this thread although I already gave my Acked-by. Below issue causes by no PG_locked page in page_referenced while page_move_anon_rmap depends on PG_locked to prevent race with rmap code. So, although this patch fixes below one example, we still have a problem in rmap. If page_referenced holds PG_locked for all of pages unconditionally, we don't need this patch and might remove READ_ONCE introduced by 80e148 and more than. What do you think about? 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 > Cc: "Paul E. McKenney" > Cc: "Kirill A. Shutemov" > Cc: Rik van Riel > Cc: Hugh Dickins > --- > Changes in v2: > - do not add READ_ONCE to PageAnon and WRITE_ONCE to > __page_set_anon_rmap and __hugepage_set_anon_rmap (Kirill) > > mm/rmap.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/rmap.c b/mm/rmap.c > index 24dd3f9fee27..8b18fd4227d1 100644 > --- a/mm/rmap.c > +++ b/mm/rmap.c > @@ -950,7 +950,7 @@ void page_move_anon_rmap(struct page *page, > VM_BUG_ON_PAGE(page->index != linear_page_index(vma, address), page); > > anon_vma = (void *) anon_vma + PAGE_MAPPING_ANON; > - page->mapping = (struct address_space *) anon_vma; > + WRITE_ONCE(page->mapping, (struct address_space *) anon_vma); > } > > /** > -- > 1.7.10.4 > > -- > 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: email@kvack.org -- Kind regards, Minchan Kim -- 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: email@kvack.org