From: Hugh Dickins <hughd@google.com> To: Linus Torvalds <torvalds@linux-foundation.org> Cc: Andrew Morton <akpm@linux-foundation.org>, Jan Stancek <jstancek@redhat.com>, Jakub Jelinek <jakub@redhat.com>, David Rientjes <rientjes@google.com>, Johannes Weiner <hannes@cmpxchg.org>, "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>, Ian Lance Taylor <iant@google.com>, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: [PATCH] mm: prevent mmap_cache race in find_vma() Date: Thu, 4 Apr 2013 11:35:10 -0700 (PDT) [thread overview] Message-ID: <alpine.LNX.2.00.1304041120510.26822@eggly.anvils> (raw) From: Jan Stancek <jstancek@redhat.com> find_vma() can be called by multiple threads with read lock held on mm->mmap_sem and any of them can update mm->mmap_cache. Prevent compiler from re-fetching mm->mmap_cache, because other readers could update it in the meantime: thread 1 thread 2 | find_vma() | find_vma() struct vm_area_struct *vma = NULL; | vma = mm->mmap_cache; | if (!(vma && vma->vm_end > addr | && vma->vm_start <= addr)) { | | mm->mmap_cache = vma; return vma; | ^^ compiler may optimize this | local variable out and re-read | mm->mmap_cache | This issue can be reproduced with gcc-4.8.0-1 on s390x by running mallocstress testcase from LTP, which triggers: kernel BUG at mm/rmap.c:1088! Call Trace: ([<000003d100c57000>] 0x3d100c57000) [<000000000023a1c0>] do_wp_page+0x2fc/0xa88 [<000000000023baae>] handle_pte_fault+0x41a/0xac8 [<000000000023d832>] handle_mm_fault+0x17a/0x268 [<000000000060507a>] do_protection_exception+0x1e2/0x394 [<0000000000603a04>] pgm_check_handler+0x138/0x13c [<000003fffcf1f07a>] 0x3fffcf1f07a Last Breaking-Event-Address: [<000000000024755e>] page_add_new_anon_rmap+0xc2/0x168 Thanks to Jakub Jelinek for his insight on gcc and helping to track this down. Signed-off-by: Jan Stancek <jstancek@redhat.com> Acked-by: David Rientjes <rientjes@google.com> Signed-off-by: Hugh Dickins <hughd@google.com> Cc: stable@vger.kernel.org --- This is Jan's valuable patch, posted a couple of days ago, which then triggered "some discussion" of ACCESS_ONCE() on linux-mm. I've marked it for stable: should go back years, but 3.5 changed the indentation. mm/mmap.c | 2 +- mm/nommu.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/mm/mmap.c b/mm/mmap.c index 6466699..0db0de1 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -1940,7 +1940,7 @@ struct vm_area_struct *find_vma(struct mm_struct *mm, unsigned long addr) /* Check the cache first. */ /* (Cache hit rate is typically around 35%.) */ - vma = mm->mmap_cache; + vma = ACCESS_ONCE(mm->mmap_cache); if (!(vma && vma->vm_end > addr && vma->vm_start <= addr)) { struct rb_node *rb_node; diff --git a/mm/nommu.c b/mm/nommu.c index e193280..2f3ea74 100644 --- a/mm/nommu.c +++ b/mm/nommu.c @@ -821,7 +821,7 @@ struct vm_area_struct *find_vma(struct mm_struct *mm, unsigned long addr) struct vm_area_struct *vma; /* check the cache first */ - vma = mm->mmap_cache; + vma = ACCESS_ONCE(mm->mmap_cache); if (vma && vma->vm_start <= addr && vma->vm_end > addr) return vma; -- 1.7.1
WARNING: multiple messages have this Message-ID (diff)
From: Hugh Dickins <hughd@google.com> To: Linus Torvalds <torvalds@linux-foundation.org> Cc: Andrew Morton <akpm@linux-foundation.org>, Jan Stancek <jstancek@redhat.com>, Jakub Jelinek <jakub@redhat.com>, David Rientjes <rientjes@google.com>, Johannes Weiner <hannes@cmpxchg.org>, "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>, Ian Lance Taylor <iant@google.com>, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: [PATCH] mm: prevent mmap_cache race in find_vma() Date: Thu, 4 Apr 2013 11:35:10 -0700 (PDT) [thread overview] Message-ID: <alpine.LNX.2.00.1304041120510.26822@eggly.anvils> (raw) From: Jan Stancek <jstancek@redhat.com> find_vma() can be called by multiple threads with read lock held on mm->mmap_sem and any of them can update mm->mmap_cache. Prevent compiler from re-fetching mm->mmap_cache, because other readers could update it in the meantime: thread 1 thread 2 | find_vma() | find_vma() struct vm_area_struct *vma = NULL; | vma = mm->mmap_cache; | if (!(vma && vma->vm_end > addr | && vma->vm_start <= addr)) { | | mm->mmap_cache = vma; return vma; | ^^ compiler may optimize this | local variable out and re-read | mm->mmap_cache | This issue can be reproduced with gcc-4.8.0-1 on s390x by running mallocstress testcase from LTP, which triggers: kernel BUG at mm/rmap.c:1088! Call Trace: ([<000003d100c57000>] 0x3d100c57000) [<000000000023a1c0>] do_wp_page+0x2fc/0xa88 [<000000000023baae>] handle_pte_fault+0x41a/0xac8 [<000000000023d832>] handle_mm_fault+0x17a/0x268 [<000000000060507a>] do_protection_exception+0x1e2/0x394 [<0000000000603a04>] pgm_check_handler+0x138/0x13c [<000003fffcf1f07a>] 0x3fffcf1f07a Last Breaking-Event-Address: [<000000000024755e>] page_add_new_anon_rmap+0xc2/0x168 Thanks to Jakub Jelinek for his insight on gcc and helping to track this down. Signed-off-by: Jan Stancek <jstancek@redhat.com> Acked-by: David Rientjes <rientjes@google.com> Signed-off-by: Hugh Dickins <hughd@google.com> Cc: stable@vger.kernel.org --- This is Jan's valuable patch, posted a couple of days ago, which then triggered "some discussion" of ACCESS_ONCE() on linux-mm. I've marked it for stable: should go back years, but 3.5 changed the indentation. mm/mmap.c | 2 +- mm/nommu.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/mm/mmap.c b/mm/mmap.c index 6466699..0db0de1 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -1940,7 +1940,7 @@ struct vm_area_struct *find_vma(struct mm_struct *mm, unsigned long addr) /* Check the cache first. */ /* (Cache hit rate is typically around 35%.) */ - vma = mm->mmap_cache; + vma = ACCESS_ONCE(mm->mmap_cache); if (!(vma && vma->vm_end > addr && vma->vm_start <= addr)) { struct rb_node *rb_node; diff --git a/mm/nommu.c b/mm/nommu.c index e193280..2f3ea74 100644 --- a/mm/nommu.c +++ b/mm/nommu.c @@ -821,7 +821,7 @@ struct vm_area_struct *find_vma(struct mm_struct *mm, unsigned long addr) struct vm_area_struct *vma; /* check the cache first */ - vma = mm->mmap_cache; + vma = ACCESS_ONCE(mm->mmap_cache); if (vma && vma->vm_start <= addr && vma->vm_end > addr) return vma; -- 1.7.1 -- 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>
next reply other threads:[~2013-04-04 18:35 UTC|newest] Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top 2013-04-04 18:35 Hugh Dickins [this message] 2013-04-04 18:35 ` [PATCH] mm: prevent mmap_cache race in find_vma() Hugh Dickins 2013-04-04 18:48 ` Linus Torvalds 2013-04-04 18:48 ` Linus Torvalds 2013-04-04 19:01 ` Hugh Dickins 2013-04-04 19:01 ` Hugh Dickins 2013-04-04 19:10 ` Linus Torvalds 2013-04-04 19:10 ` Linus Torvalds 2013-04-04 22:30 ` Paul E. McKenney 2013-04-04 22:30 ` Paul E. McKenney -- strict thread matches above, loose matches on Subject: below -- 2013-04-02 21:59 Jan Stancek 2013-04-02 22:33 ` David Rientjes 2013-04-02 23:09 ` Hugh Dickins 2013-04-02 23:55 ` David Rientjes 2013-04-03 3:19 ` Paul E. McKenney 2013-04-03 4:21 ` David Rientjes 2013-04-03 16:38 ` Paul E. McKenney 2013-04-03 4:14 ` Johannes Weiner 2013-04-03 4:25 ` David Rientjes 2013-04-03 4:58 ` Johannes Weiner 2013-04-03 5:13 ` David Rientjes 2013-04-03 13:45 ` Ian Lance Taylor 2013-04-03 14:33 ` Johannes Weiner 2013-04-03 23:59 ` David Rientjes 2013-04-03 16:33 ` Paul E. McKenney 2013-04-03 16:41 ` Paul E. McKenney 2013-04-03 17:47 ` Ian Lance Taylor 2013-04-03 22:11 ` Paul E. McKenney 2013-04-03 22:28 ` Ian Lance Taylor 2013-04-12 18:05 ` Paul E. McKenney 2013-04-03 9:37 ` Jakub Jelinek
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=alpine.LNX.2.00.1304041120510.26822@eggly.anvils \ --to=hughd@google.com \ --cc=akpm@linux-foundation.org \ --cc=hannes@cmpxchg.org \ --cc=iant@google.com \ --cc=jakub@redhat.com \ --cc=jstancek@redhat.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-mm@kvack.org \ --cc=paulmck@linux.vnet.ibm.com \ --cc=rientjes@google.com \ --cc=torvalds@linux-foundation.org \ /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: linkBe 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.