All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
To: David Rientjes <rientjes@google.com>
Cc: Hugh Dickins <hughd@google.com>,
	Jan Stancek <jstancek@redhat.com>,
	Ian Lance Taylor <iant@google.com>,
	linux-mm@kvack.org
Subject: Re: [PATCH] mm: prevent mmap_cache race in find_vma()
Date: Wed, 3 Apr 2013 09:38:23 -0700	[thread overview]
Message-ID: <20130403163823.GE28522@linux.vnet.ibm.com> (raw)
In-Reply-To: <alpine.DEB.2.02.1304022110160.32184@chino.kir.corp.google.com>

On Tue, Apr 02, 2013 at 09:21:49PM -0700, David Rientjes wrote:
> On Tue, 2 Apr 2013, Paul E. McKenney wrote:
> 
> > So although I agree that the standard does not say as much as one might
> > like about volatile, ACCESS_ONCE()'s use of volatile should be expected
> > to work in a wide range of C compilers.  ACCESS_ONCE()'s use of typeof()
> > might not be quite so generally applicable, but a fair range of C
> > compilers do seem to support typeof() as well as ACCESS_ONCE()'s use
> > of volatile.
> > 
> 
> Agreed and I have nothing against code that uses it in that manner based 
> on the implementations of those compilers.  The _only_ thing I've said in 
> this thread is that ACCESS_ONCE() does not "prevent the compiler from 
> re-fetching."  The only thing that is going to prevent the compiler from 
> doing anything is the standard and, as you eluded, it's legal for a 
> compiler to compile code such as 
> 
> 	vma = ACCESS_ONCE(mm->mmap_cache);
> 	if (vma && vma->vm_start <= addr && vma->vm_end > addr)
> 		return vma;
> 
> to be equivalent as if it had been written
> 
> 	if (mm->mmap_cache && mm->mmap_cache->vm_start <= addr &&
> 	    mm->mmap_cache->vm_end > addr)
> 		return mm->mmap_cache;
> 
> and still be a conforming implementation.  We know gcc doesn't do that, so 
> nobody is arguing the code in this patch as being incorrect.  In fact, to 
> remove any question about it:
> 
> Acked-by: David Rientjes <rientjes@google.com>

Thank you!

> However, as originally stated, I would prefer that the changelog be 
> reworded so nobody believes ACCESS_ONCE() prevents the compiler from 
> re-fetching anything.

If you were to instead say:

	However, as originally stated, I would prefer that the changelog
	be reworded so nobody believes that the C standard guarantees that
	volatile casts prevent the compiler from re-fetching anything.

I might agree with you.  But ACCESS_ONCE() really is defined to prevent
the compiler from refetching anything.  If a new version of gcc appears
for which volatile casts does not protect against refetching, then we
will change either (1) gcc or (2) the implementation of ACCESS_ONCE().
Whatever is needed to provide the guarantee against refetching.  The
Linux kernel absolutely needs -something- that provides this guarantee.

							Thanx, Paul

--
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-03 16:38 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-04-02 21:59 [PATCH] mm: prevent mmap_cache race in find_vma() 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 [this message]
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-04  0:00                   ` [patch] compiler: clarify ACCESS_ONCE() relies on compiler implementation David Rientjes
2013-04-04  0:38                     ` Linus Torvalds
2013-04-04  1:52                       ` David Rientjes
2013-04-04  2:00                         ` Linus Torvalds
2013-04-04  2:18                           ` David Rientjes
2013-04-04  2:37                             ` Linus Torvalds
2013-04-04  6:02                               ` David Rientjes
2013-04-04 14:23                                 ` Linus Torvalds
2013-04-04 19:40                                   ` David Rientjes
2013-04-04 19:53                                     ` Linus Torvalds
2013-04-04 20:02                                       ` David Rientjes
2013-04-03 16:33               ` [PATCH] mm: prevent mmap_cache race in find_vma() 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
2013-04-04 18:35 Hugh Dickins
2013-04-04 18:35 ` 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

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=20130403163823.GE28522@linux.vnet.ibm.com \
    --to=paulmck@linux.vnet.ibm.com \
    --cc=hughd@google.com \
    --cc=iant@google.com \
    --cc=jstancek@redhat.com \
    --cc=linux-mm@kvack.org \
    --cc=rientjes@google.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.