All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Chris Snook <csnook@redhat.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>,
	andi@firstfloor.org, David Miller <davem@davemloft.net>,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	schwidefsky@de.ibm.com, wensong@linux-vs.org, horms@verge.net.au
Subject: Re: [patch] ipvs: force read of atomic_t in while loop
Date: Wed, 8 Aug 2007 14:31:15 -0700	[thread overview]
Message-ID: <20070808143115.de539511.akpm@linux-foundation.org> (raw)
In-Reply-To: <46BA30DC.20207@redhat.com>

On Wed, 08 Aug 2007 17:08:44 -0400
Chris Snook <csnook@redhat.com> wrote:

> Heiko Carstens wrote:
> > On Wed, Aug 08, 2007 at 03:21:31AM -0700, David Miller wrote:
> >> From: Heiko Carstens <heiko.carstens@de.ibm.com>
> >> Date: Wed, 8 Aug 2007 11:33:00 +0200
> >>
> >>> Just saw this while grepping for atomic_reads in a while loops.
> >>> Maybe we should re-add the volatile to atomic_t. Not sure.
> >> I think whatever the choice, it should be done consistently
> >> on every architecture.
> >>
> >> It's just asking for trouble if your arch does it differently from
> >> every other.
> > 
> > Well..currently it's i386/x86_64 and s390 which have no volatile
> > in atomic_t. And yes, of course I agree it should be consistent
> > across all architectures. But it isn't.
> 
> Based on recent discussion, it's pretty clear that there's a lot of 
> confusion about this.  A lot of people (myself included, until I thought 
> about it long and hard) will reasonably assume that calling 
> atomic_read() will actually read the value from memory.  Leaving out the 
> volatile declaration seems like a pessimization to me.  If you force 
> people to use barrier() everywhere they're working with atomic_t, it 
> will force re-reads of all the non-atomic data in use as well, which 
> will cause more memory fetches of things that generally don't need 
> barrier().  That and it's a bug waiting to happen.
> 
> Andi -- your thoughts on the matter?

I'm not Andi, but this not-Andi thinks that permitting the compiler to cache
the results of atomic_read() is dumb.


  reply	other threads:[~2007-08-08 21:37 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-08-08  9:33 [patch] ipvs: force read of atomic_t in while loop Heiko Carstens
2007-08-08  9:45 ` Horms
2007-08-08 10:21 ` David Miller
2007-08-08 10:28   ` Heiko Carstens
2007-08-08 21:08     ` Chris Snook
2007-08-08 21:31       ` Andrew Morton [this message]
2007-08-08 22:27         ` Heiko Carstens
2007-08-08 22:38           ` Chris Snook
2007-08-09  0:15       ` Andi Kleen
2007-08-09 12:35         ` Michael Buesch
2007-08-09 12:40           ` Chris Snook
2007-08-09 12:49             ` Martin Schwidefsky
2007-08-09 13:36           ` Andi Kleen

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=20070808143115.de539511.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=andi@firstfloor.org \
    --cc=csnook@redhat.com \
    --cc=davem@davemloft.net \
    --cc=heiko.carstens@de.ibm.com \
    --cc=horms@verge.net.au \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=schwidefsky@de.ibm.com \
    --cc=wensong@linux-vs.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 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.