linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
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 <linux-mm@kvack.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] mm: prevent mmap_cache race in find_vma()
Date: Thu, 4 Apr 2013 12:01:52 -0700 (PDT)	[thread overview]
Message-ID: <alpine.LNX.2.00.1304041149030.29847@eggly.anvils> (raw)
In-Reply-To: <CA+55aFwCG2h1ijWTCJ38LVcUyczDAfk72c4MVSU+_-BiLoMOOw@mail.gmail.com>

On Thu, 4 Apr 2013, Linus Torvalds wrote:
> On Thu, Apr 4, 2013 at 11:35 AM, Hugh Dickins <hughd@google.com> wrote:
> >
> > 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:
> 
> Ack. I do wonder if we should mark the unlocked update too some way
> (also in find_vma()), although it's probably not a problem in practice
> since there's no way the compiler can reasonably really do anything
> odd with it. We *could* make that an ACCESS_ONCE() write too just to
> highlight the fact that it's an unlocked write to this optimistic data
> structure.

Hah, you beat me to it.

I wanted to get Jan's patch in first, seeing as it actually fixes his
observed issue; and it is very nice to have such a good description of
one of those, when ACCESS_ONCE() is usually just an insurance policy.

But then I was researching the much rarer "ACCESS_ONCE(x) = y" usage
(popular in drivers/net/wireless/ath/ath9k and kernel/rcutree* and
sound/firewire, but few places else).

When Paul reminded us of it yesterday, I came to wonder if actually
every use of ACCESS_ONCE in the read form should strictly be matched
by ACCESS_ONCE whenever modifying the location.

My uneducated guess is that strictly it ought to, in the sense of
insurance policy; but that (apart from that strange split writing
issue which came up a couple of months ago) in practice our compilers
have not "advanced" to the point of making this an issue yet.

> 
> Anyway, applied.

Thanks,
Hugh

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

  reply	other threads:[~2013-04-04 19:02 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-04 18:35 Hugh Dickins
2013-04-04 18:48 ` Linus Torvalds
2013-04-04 19:01   ` Hugh Dickins [this message]
2013-04-04 19:10     ` Linus Torvalds
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.1304041149030.29847@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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox