* Re: [patch] ipvs: force read of atomic_t in while loop
From: Michael Buesch @ 2007-08-09 12:35 UTC (permalink / raw)
To: Andi Kleen
Cc: Chris Snook, Heiko Carstens, David Miller, akpm, linux-kernel,
netdev, schwidefsky, wensong, horms, torvalds
In-Reply-To: <20070809001533.GA17798@one.firstfloor.org>
On Thursday 09 August 2007 02:15:33 Andi Kleen wrote:
> On Wed, Aug 08, 2007 at 05:08:44PM -0400, Chris Snook 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 also think readding volatile makes sense. An alternative would be
> to stick an rmb() into atomic_read() -- that would also stop speculative reads.
> Disadvantage is that it clobbers all memory, not just the specific value.
>
> But you really have to complain to Linus (cc'ed). He came up
> with the volatile removale change iirc.
Isn't it possible through some inline assembly trick
that only a certain variable has to be reloaded?
So we could define something like that:
#define reload_var(x) __asm__ __volatile__ (whatever, x)
I don't know inline assembly that much, but isn't it possible
with that to kind of "fake-touch" the variable, so the compiler
must reload it (and only it) to make sure it's up to date?
--
Greetings Michael.
^ permalink raw reply
* Re: [patch] ipvs: force read of atomic_t in while loop
From: Chris Snook @ 2007-08-09 12:40 UTC (permalink / raw)
To: Michael Buesch
Cc: Andi Kleen, Heiko Carstens, David Miller, akpm, linux-kernel,
netdev, schwidefsky, wensong, horms, torvalds
In-Reply-To: <200708091435.18595.mb@bu3sch.de>
Michael Buesch wrote:
> On Thursday 09 August 2007 02:15:33 Andi Kleen wrote:
>> On Wed, Aug 08, 2007 at 05:08:44PM -0400, Chris Snook 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 also think readding volatile makes sense. An alternative would be
>> to stick an rmb() into atomic_read() -- that would also stop speculative reads.
>> Disadvantage is that it clobbers all memory, not just the specific value.
>>
>> But you really have to complain to Linus (cc'ed). He came up
>> with the volatile removale change iirc.
>
> Isn't it possible through some inline assembly trick
> that only a certain variable has to be reloaded?
> So we could define something like that:
>
> #define reload_var(x) __asm__ __volatile__ (whatever, x)
>
> I don't know inline assembly that much, but isn't it possible
> with that to kind of "fake-touch" the variable, so the compiler
> must reload it (and only it) to make sure it's up to date?
We can do it in C, like this:
-#define atomic_read(v) ((v)->counter)
+#define atomic_read(v) (*(volatile int *)&(v)->counter)
By casting it volatile at the precise piece of code where we want to guarantee a
read from memory, there's little risk of the compiler getting creative in its
interpretation of the code.
Stay tuned for the patch set...
-- Chris
^ permalink raw reply
* Re: [patch] ipvs: force read of atomic_t in while loop
From: Martin Schwidefsky @ 2007-08-09 12:49 UTC (permalink / raw)
To: Chris Snook
Cc: Michael Buesch, Andi Kleen, Heiko Carstens, David Miller, akpm,
linux-kernel, netdev, wensong, horms, torvalds
In-Reply-To: <46BB0B4B.4070300@redhat.com>
On Thu, 2007-08-09 at 08:40 -0400, Chris Snook wrote:
> > #define reload_var(x) __asm__ __volatile__ (whatever, x)
> >
> > I don't know inline assembly that much, but isn't it possible
> > with that to kind of "fake-touch" the variable, so the compiler
> > must reload it (and only it) to make sure it's up to date?
>
> We can do it in C, like this:
>
> -#define atomic_read(v) ((v)->counter)
> +#define atomic_read(v) (*(volatile int *)&(v)->counter)
>
> By casting it volatile at the precise piece of code where we want to
> guarantee a read from memory, there's little risk of the compiler
> getting creative in its interpretation of the code.
To answer the inline assembler question:
asm volatile ("" : "=m" (counter)) : "m" (counter) )
will force the compiler to reload the value from memory. But the cast to
(volatile int *) is even better.
--
blue skies,
Martin.
"Reality continues to ruin my life." - Calvin.
^ permalink raw reply
* [PATCH] Fix the SSB dependency hell
From: Michael Buesch @ 2007-08-09 13:04 UTC (permalink / raw)
To: John Linville; +Cc: netdev, linux-wireless, Andrew Morton
This fixes the SSB dependency hell and introduces some
fake-options that only give some advice on what to select.
We live with these fake options only until menuconfig is able
to tell more about needed dependencies and how to resolve them.
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Michael Buesch <mb@bu3sch.de>
Index: wireless-dev/drivers/net/Kconfig
===================================================================
--- wireless-dev.orig/drivers/net/Kconfig 2007-08-09 14:47:01.000000000 +0200
+++ wireless-dev/drivers/net/Kconfig 2007-08-09 14:47:40.000000000 +0200
@@ -1452,10 +1452,31 @@ config APRICOT
<file:Documentation/networking/net-modules.txt>. The module will be
called apricot.
+config B44_DEP_HACK
+ bool
+ depends on SSB && SSB_PCIHOST && SSB_DRIVER_PCICORE
+ default y
+
+config B44_ADVICE_HACK
+ bool "B44 for PCI not available. Read the help text of this option!"
+ depends on !B44_DEP_HACK
+ ---help---
+ The Broadcom 440x/47xx driver for PCI devices can not be enabled,
+ because the required dependencies are not selected.
+
+ In order to be able to select the Broadcom 440x/47xx PCI driver, you
+ need to enable the following options first:
+
+ CONFIG_SSB found in menu:
+ Device Drivers/Sonics Silicon Backplane/Sonics Silicon Backplane support
+ CONFIG_SSB_PCIHOST found in menu:
+ Device Drivers/Sonics Silicon Backplane/Support for SSB on PCI-bus host
+ CONFIG_SSB_DRIVER_PCICORE found in menu:
+ Device Drivers/Sonics Silicon Backplane/SSB PCI core driver
+
config B44
tristate "Broadcom 440x/47xx ethernet support"
- depends on HAS_IOMEM
- select SSB
+ depends on SSB
select MII
help
If you have a network (Ethernet) controller of this type, say Y
@@ -1473,9 +1494,7 @@ config B44
config B44_PCI
bool "Broadcom 440x PCI device support"
- depends on B44 && NET_PCI
- select SSB_PCIHOST
- select SSB_DRIVER_PCICORE
+ depends on B44 && SSB_PCIHOST && SSB_DRIVER_PCICORE && NET_PCI
default y
help
Support for Broadcom 440x PCI devices.
Index: wireless-dev/drivers/net/wireless/bcm43xx-mac80211/Kconfig
===================================================================
--- wireless-dev.orig/drivers/net/wireless/bcm43xx-mac80211/Kconfig 2007-08-09 14:47:01.000000000 +0200
+++ wireless-dev/drivers/net/wireless/bcm43xx-mac80211/Kconfig 2007-08-09 14:59:39.000000000 +0200
@@ -1,8 +1,29 @@
+config BCM43XX_DEP_HACK
+ bool
+ depends on SSB && SSB_PCIHOST && SSB_DRIVER_PCICORE
+ default y
+
+config BCM43XX_ADVICE_HACK
+ bool "BCM43xx PCI (mac80211) not available. Read the help text of this option!"
+ depends on !BCM43XX_DEP_HACK
+ ---help---
+ The BCM43xx driver for BCM43xx PCI devices can not be enabled,
+ because the required dependencies are not selected.
+
+ In order to be able to select the BCM43xx-mac80211 driver, you
+ need to enable the following options first:
+
+ CONFIG_SSB found in menu:
+ Device Drivers/Sonics Silicon Backplane/Sonics Silicon Backplane support
+ CONFIG_SSB_PCIHOST found in menu:
+ Device Drivers/Sonics Silicon Backplane/Support for SSB on PCI-bus host
+ CONFIG_SSB_DRIVER_PCICORE found in menu:
+ Device Drivers/Sonics Silicon Backplane/SSB PCI core driver
+
config BCM43XX_MAC80211
tristate "Broadcom BCM43xx wireless support (mac80211 stack)"
- depends on MAC80211 && WLAN_80211 && EXPERIMENTAL
+ depends on SSB && MAC80211 && WLAN_80211 && EXPERIMENTAL
select FW_LOADER
- select SSB
select HW_RANDOM
---help---
This is an experimental driver for the Broadcom 43xx wireless chip,
@@ -10,9 +31,7 @@ config BCM43XX_MAC80211
config BCM43XX_MAC80211_PCI
bool "BCM43xx PCI device support"
- depends on BCM43XX_MAC80211 && PCI
- select SSB_PCIHOST
- select SSB_DRIVER_PCICORE
+ depends on BCM43XX_MAC80211 && SSB_PCIHOST && SSB_DRIVER_PCICORE
default y
---help---
Broadcom 43xx PCI device support.
@@ -24,8 +43,7 @@ config BCM43XX_MAC80211_PCI
config BCM43XX_MAC80211_PCMCIA
bool "BCM43xx PCMCIA device support"
- depends on BCM43XX_MAC80211 && PCMCIA
- select SSB_PCMCIAHOST
+ depends on BCM43XX_MAC80211 && SSB_PCMCIAHOST
---help---
Broadcom 43xx PCMCIA device support.
@@ -43,14 +61,13 @@ config BCM43XX_MAC80211_PCMCIA
If unsure, say N.
config BCM43XX_MAC80211_DEBUG
- bool "Broadcom BCM43xx debugging (RECOMMENDED)"
+ bool "Broadcom BCM43xx debugging"
depends on BCM43XX_MAC80211
- select SSB_DEBUG if !SSB_SILENT
- default y
---help---
Broadcom 43xx debugging messages.
- Say Y, because the driver is still very experimental and
- this will help you get it running.
+
+ Say Y, if you want to find out why the driver does not
+ work for you.
config BCM43XX_MAC80211_DMA
bool
Index: wireless-dev/drivers/ssb/Kconfig
===================================================================
--- wireless-dev.orig/drivers/ssb/Kconfig 2007-08-09 14:47:01.000000000 +0200
+++ wireless-dev/drivers/ssb/Kconfig 2007-08-09 14:51:09.000000000 +0200
@@ -2,7 +2,7 @@ menu "Sonics Silicon Backplane"
config SSB
tristate "Sonics Silicon Backplane support"
- depends on EXPERIMENTAL && HAS_IOMEM
+ depends on HAS_IOMEM
help
Support for the Sonics Silicon Backplane bus
@@ -21,8 +21,8 @@ config SSB_PCIHOST
If unsure, say Y
config SSB_PCMCIAHOST
- bool "Support for SSB on PCMCIA-bus host"
- depends on SSB && PCMCIA
+ bool "Support for SSB on PCMCIA-bus host (EXPERIMENTAL)"
+ depends on SSB && PCMCIA && EXPERIMENTAL
help
Support for a Sonics Silicon Backplane on top
of a PCMCIA device.
@@ -65,14 +65,14 @@ config SSB_DRIVER_PCICORE
If unsure, say Y
config SSB_PCICORE_HOSTMODE
- bool "Hostmode support for SSB PCI core"
- depends on SSB_DRIVER_PCICORE && SSB_DRIVER_MIPS
+ bool "Hostmode support for SSB PCI core (EXPERIMENTAL)"
+ depends on SSB_DRIVER_PCICORE && SSB_DRIVER_MIPS && EXPERIMENTAL
help
PCIcore hostmode operation (external PCI bus).
config SSB_DRIVER_MIPS
- bool "SSB Broadcom MIPS core driver"
- depends on SSB && MIPS
+ bool "SSB Broadcom MIPS core driver (EXPERIMENTAL)"
+ depends on SSB && MIPS && EXPERIMENTAL
select SSB_SERIAL
help
Driver for the Sonics Silicon Backplane attached
@@ -81,8 +81,8 @@ config SSB_DRIVER_MIPS
If unsure, say N
config SSB_DRIVER_EXTIF
- bool "SSB Broadcom EXTIF core driver"
- depends on SSB_DRIVER_MIPS
+ bool "SSB Broadcom EXTIF core driver (EXPERIMENTAL)"
+ depends on SSB_DRIVER_MIPS && EXPERIMENTAL
help
Driver for the Sonics Silicon Backplane attached
Broadcom EXTIF core.
^ permalink raw reply
* [PATCH 0/24] make atomic_read() behave consistently across all architectures
From: Chris Snook @ 2007-08-09 13:14 UTC (permalink / raw)
To: linux-kernel, linux-arch, torvalds
Cc: netdev, akpm, ak, heiko.carstens, davem, schwidefsky, wensong,
horms, wjiang, cfriesen, zlynx, rpjday, jesper.juhl
As recent discussions[1], and bugs[2] have shown, there is a great deal of
confusion about the expected behavior of atomic_read(), compounded by the
fact that it is not the same on all architectures. Since users expect calls
to atomic_read() to actually perform a read, it is not desirable to allow
the compiler to optimize this away. Requiring the use of barrier() in this
case is inefficient, since we only want to re-load the atomic_t variable,
not everything else in scope.
This patchset makes the behavior of atomic_read uniform by removing the
volatile keyword from all atomic_t and atomic64_t definitions that currently
have it, and instead explicitly casts the variable as volatile in
atomic_read(). This leaves little room for creative optimization by the
compiler, and is in keeping with the principles behind "volatile considered
harmful".
Busy-waiters should still use cpu_relax(), but fast paths may be able to
reduce their use of barrier() between some atomic_read() calls.
-- Chris
1) http://lkml.org/lkml/2007/7/1/52
2) http://lkml.org/lkml/2007/8/8/122
^ permalink raw reply
* Re: 2.6.23-rc2-mm1
From: Michal Piotrowski @ 2007-08-09 13:23 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel, Netdev, Jeff Garzik
In-Reply-To: <20070809015106.cd0bfc53.akpm@linux-foundation.org>
Andrew Morton pisze:
> ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.23-rc2/2.6.23-rc2-mm1/
I am experiencing some problems with 8139too
[ 28.847004] 8139too 0000:02:0d.0: region #0 not a PIO resource, aborting
[ 28.854722] Bad IO access at port 0 ()
[ 28.859459] WARNING: at /home/devel/linux-mm/lib/iomap.c:44 bad_io_access()
[ 28.867415] [<c040536b>] show_trace_log_lvl+0x1a/0x30
[ 28.873568] [<c0405ff3>] show_trace+0x12/0x14
[ 28.879015] [<c0406128>] dump_stack+0x16/0x18
[ 28.884451] [<c052a904>] bad_io_access+0x58/0x5a
[ 28.890129] [<c052a927>] pci_iounmap+0x21/0x2b
[ 28.895635] [<c05b3746>] __rtl8139_cleanup_dev+0x75/0xc6
[ 28.902037] [<c05b42c4>] rtl8139_init_one+0x59b/0xa9f
[ 28.908170] [<c053e344>] pci_device_probe+0x44/0x5f
[ 28.914116] [<c05a7cd0>] driver_probe_device+0xa7/0x19a
[ 28.920402] [<c05a7f1a>] __driver_attach+0xa6/0xa8
[ 28.926236] [<c05a71d3>] bus_for_each_dev+0x43/0x61
[ 28.932139] [<c05a7b51>] driver_attach+0x19/0x1b
[ 28.937776] [<c05a7504>] bus_add_driver+0x7e/0x1a5
[ 28.943567] [<c05a80cf>] driver_register+0x45/0x75
[ 28.949358] [<c053e4a5>] __pci_register_driver+0x56/0x84
[ 28.955678] [<c0819118>] rtl8139_init_module+0x14/0x1c
[ 28.961832] [<c0800521>] kernel_init+0x132/0x306
[ 28.967451] [<c0404fb3>] kernel_thread_helper+0x7/0x14
[ 28.973588] =======================
[ 28.978151] initcall 0xc0819104: rtl8139_init_module+0x0/0x1c() returned 0.
[ 28.986114] initcall 0xc0819104 ran for 161 msecs: rtl8139_init_module+0x0/0x1c()
http://www.stardust.webpages.pl/files/tbf/bitis-gabonica/2.6.23-rc2-mm1/mm-dmesg
http://www.stardust.webpages.pl/files/tbf/bitis-gabonica/2.6.23-rc2-mm1/mm-config
Regards,
Michal
--
LOG
http://www.stardust.webpages.pl/log/
^ permalink raw reply
* [PATCH 1/24] make atomic_read() behave consistently on alpha
From: Chris Snook @ 2007-08-09 13:24 UTC (permalink / raw)
To: linux-kernel, linux-arch, torvalds
Cc: netdev, akpm, ak, heiko.carstens, davem, schwidefsky, wensong,
horms, wjiang, cfriesen, zlynx, rpjday, jesper.juhl
From: Chris Snook <csnook@redhat.com>
Purify volatile use for atomic[64]_t on alpha.
Signed-off-by: Chris Snook <csnook@redhat.com>
--- linux-2.6.23-rc2-orig/include/asm-alpha/atomic.h 2007-07-08 19:32:17.000000000 -0400
+++ linux-2.6.23-rc2/include/asm-alpha/atomic.h 2007-08-09 09:19:00.000000000 -0400
@@ -14,18 +14,22 @@
/*
- * Counter is volatile to make sure gcc doesn't try to be clever
- * and move things around on us. We need to use _exactly_ the address
- * the user gave us, not some alias that contains the same information.
+ * Make sure gcc doesn't try to be clever and move things around
+ * on us. We need to use _exactly_ the address the user gave us,
+ * not some alias that contains the same information.
*/
-typedef struct { volatile int counter; } atomic_t;
-typedef struct { volatile long counter; } atomic64_t;
+typedef struct { int counter; } atomic_t;
+typedef struct { long counter; } atomic64_t;
#define ATOMIC_INIT(i) ( (atomic_t) { (i) } )
#define ATOMIC64_INIT(i) ( (atomic64_t) { (i) } )
-#define atomic_read(v) ((v)->counter + 0)
-#define atomic64_read(v) ((v)->counter + 0)
+/*
+ * Casting to volatile here minimizes the need for barriers,
+ * without having to declare the type itself as volatile.
+ */
+#define atomic_read(v) (*(volatile int *)&(v)->counter + 0)
+#define atomic64_read(v) (*(volatile long *)&(v)->counter + 0)
#define atomic_set(v,i) ((v)->counter = (i))
#define atomic64_set(v,i) ((v)->counter = (i))
^ permalink raw reply
* Re: myri10ge net-2.6.24 build fix
From: Andrew Gallatin @ 2007-08-09 13:30 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20070809.001138.39464000.davem@davemloft.net>
David Miller wrote:
> I had to add the following patch to fix the build after
> the LRO changes, I have no idea how you could have compile
> tested that patch let alone done any real testing on it :-/
Whoops. I'm very sorry about that. Future patches will be submitted
by our Linux guy, who knows the correct procedures. :)
FWIW, the patch I sent came from a script which filters our internal
driver (which I always work with), and that line was erroneously
filtered by the script we use to remove all the stuff you guys frown
on (like our LRO, compat shims for older kernels, optional support to
receive into skbs rather than pages, etc). My testing was done with
our un-filtered upstream driver.
Since our driver specific LRO is now gone (hurray!), I removed the LRO
filtering, re-ran the filter script, and arrived at the same patch
you committed.
Thanks for fixing my mistake!
Drew
^ permalink raw reply
* [PATCH 2/24] make atomic_read() behave consistently on arm
From: Chris Snook @ 2007-08-09 13:30 UTC (permalink / raw)
To: linux-kernel, linux-arch, torvalds
Cc: netdev, akpm, ak, heiko.carstens, davem, schwidefsky, wensong,
horms, wjiang, cfriesen, zlynx, rpjday, jesper.juhl
From: Chris Snook <csnook@redhat.com>
Purify volatile use for atomic_t on arm.
Signed-off-by: Chris Snook <csnook@redhat.com>
--- linux-2.6.23-rc2-orig/include/asm-arm/atomic.h 2007-07-08 19:32:17.000000000 -0400
+++ linux-2.6.23-rc2/include/asm-arm/atomic.h 2007-08-09 06:30:40.000000000 -0400
@@ -14,13 +14,17 @@
#include <linux/compiler.h>
#include <asm/system.h>
-typedef struct { volatile int counter; } atomic_t;
+typedef struct { int counter; } atomic_t;
#define ATOMIC_INIT(i) { (i) }
#ifdef __KERNEL__
-#define atomic_read(v) ((v)->counter)
+/*
+ * Casting to volatile here minimizes the need for barriers,
+ * without having to declare the type itself as volatile.
+ */
+#define atomic_read(v) (*(volatile int *)&(v)->counter)
#if __LINUX_ARM_ARCH__ >= 6
^ permalink raw reply
* [PATCH 3/24] make atomic_read() behave consistently on avr32
From: Chris Snook @ 2007-08-09 13:32 UTC (permalink / raw)
To: linux-kernel, linux-arch, torvalds
Cc: netdev, akpm, ak, heiko.carstens, davem, schwidefsky, wensong,
horms, wjiang, cfriesen, zlynx, rpjday, jesper.juhl
From: Chris Snook <csnook@redhat.com>
Purify volatile use for atomic_t on avr32.
Signed-off-by: Chris Snook <csnook@redhat.com>
--- linux-2.6.23-rc2-orig/include/asm-avr32/atomic.h 2007-08-08 17:48:52.000000000 -0400
+++ linux-2.6.23-rc2/include/asm-avr32/atomic.h 2007-08-09 06:33:39.000000000 -0400
@@ -16,10 +16,14 @@
#include <asm/system.h>
-typedef struct { volatile int counter; } atomic_t;
+typedef struct { int counter; } atomic_t;
#define ATOMIC_INIT(i) { (i) }
-#define atomic_read(v) ((v)->counter)
+/*
+ * Casting to volatile here minimizes the need for barriers,
+ * without having to declare the type itself as volatile.
+ */
+#define atomic_read(v) (*(volatile int *)&(v)->counter)
#define atomic_set(v, i) (((v)->counter) = i)
/*
^ permalink raw reply
* Re: [patch] ipvs: force read of atomic_t in while loop
From: Andi Kleen @ 2007-08-09 13:36 UTC (permalink / raw)
To: Michael Buesch
Cc: Andi Kleen, Chris Snook, Heiko Carstens, David Miller, akpm,
linux-kernel, netdev, schwidefsky, wensong, horms, torvalds
In-Reply-To: <200708091435.18595.mb@bu3sch.de>
> Isn't it possible through some inline assembly trick
> that only a certain variable has to be reloaded?
A volatile cast does that already
-Andi
^ permalink raw reply
* [PATCH 4/24] make atomic_read() behave consistently on blackfin
From: Chris Snook @ 2007-08-09 13:37 UTC (permalink / raw)
To: linux-kernel, linux-arch, torvalds
Cc: netdev, akpm, ak, heiko.carstens, davem, schwidefsky, wensong,
horms, wjiang, cfriesen, zlynx, rpjday, jesper.juhl
From: Chris Snook <csnook@redhat.com>
Make atomic_read() volatile on blackfin. This ensures that busy-waiting
for an interrupt handler to change an atomic_t won't get compiled to an
infinite loop, consistent with SMP architectures.
Signed-off-by: Chris Snook <csnook@redhat.com>
--- linux-2.6.23-rc2-orig/include/asm-blackfin/atomic.h 2007-07-08 19:32:17.000000000 -0400
+++ linux-2.6.23-rc2/include/asm-blackfin/atomic.h 2007-08-09 06:36:15.000000000 -0400
@@ -18,7 +18,11 @@ typedef struct {
} atomic_t;
#define ATOMIC_INIT(i) { (i) }
-#define atomic_read(v) ((v)->counter)
+/*
+ * Casting to volatile here minimizes the need for barriers,
+ * without having to declare the type itself as volatile.
+ */
+#define atomic_read(v) (*(volatile int *)&(v)->counter)
#define atomic_set(v, i) (((v)->counter) = i)
static __inline__ void atomic_add(int i, atomic_t * v)
^ permalink raw reply
* [PATCH 5/24] make atomic_read() behave consistently on cris
From: Chris Snook @ 2007-08-09 13:39 UTC (permalink / raw)
To: linux-kernel, linux-arch, torvalds
Cc: netdev, akpm, ak, heiko.carstens, davem, schwidefsky, wensong,
horms, wjiang, cfriesen, zlynx, rpjday, jesper.juhl
From: Chris Snook <csnook@redhat.com>
Purify volatile use for atomic_t on cris.
Signed-off-by: Chris Snook <csnook@redhat.com>
--- linux-2.6.23-rc2-orig/include/asm-cris/atomic.h 2007-07-08 19:32:17.000000000 -0400
+++ linux-2.6.23-rc2/include/asm-cris/atomic.h 2007-08-09 06:38:28.000000000 -0400
@@ -11,11 +11,15 @@
* resource counting etc..
*/
-typedef struct { volatile int counter; } atomic_t;
+typedef struct { int counter; } atomic_t;
#define ATOMIC_INIT(i) { (i) }
-#define atomic_read(v) ((v)->counter)
+/*
+ * Casting to volatile here minimizes the need for barriers,
+ * without having to declare the type itself as volatile.
+ */
+#define atomic_read(v) (*(volatile int *)&(v)->counter)
#define atomic_set(v,i) (((v)->counter) = (i))
/* These should be written in asm but we do it in C for now. */
^ permalink raw reply
* [PATCH 6/24] make atomic_read() behave consistently on frv
From: Chris Snook @ 2007-08-09 13:41 UTC (permalink / raw)
To: linux-kernel, linux-arch, torvalds
Cc: netdev, akpm, ak, heiko.carstens, davem, schwidefsky, wensong,
horms, wjiang, cfriesen, zlynx, rpjday, jesper.juhl
From: Chris Snook <csnook@redhat.com>
Make atomic_read() volatile on frv. This ensures that busy-waiting
for an interrupt handler to change an atomic_t won't get compiled to an
infinite loop, consistent with SMP architectures.
Signed-off-by: Chris Snook <csnook@redhat.com>
--- linux-2.6.23-rc2-orig/include/asm-frv/atomic.h 2007-07-08 19:32:17.000000000 -0400
+++ linux-2.6.23-rc2/include/asm-frv/atomic.h 2007-08-09 06:41:48.000000000 -0400
@@ -40,7 +40,12 @@ typedef struct {
} atomic_t;
#define ATOMIC_INIT(i) { (i) }
-#define atomic_read(v) ((v)->counter)
+
+/*
+ * Casting to volatile here minimizes the need for barriers,
+ * without having to declare the type itself as volatile.
+ */
+#define atomic_read(v) (*(volatile int *)&(v)->counter)
#define atomic_set(v, i) (((v)->counter) = (i))
#ifndef CONFIG_FRV_OUTOFLINE_ATOMIC_OPS
^ permalink raw reply
* [PATCH 8/24] make atomic_read() behave consistently on i386
From: Chris Snook @ 2007-08-09 13:49 UTC (permalink / raw)
To: linux-kernel, linux-arch, torvalds
Cc: netdev, akpm, ak, heiko.carstens, davem, schwidefsky, wensong,
horms, wjiang, cfriesen, zlynx, rpjday, jesper.juhl
From: Chris Snook <csnook@redhat.com>
Make atomic_read() volatile on i386, to ensure memory is actually read
each time.
Signed-off-by: Chris Snook <csnook@redhat.com>
--- linux-2.6.23-rc2-orig/include/asm-i386/atomic.h 2007-07-08 19:32:17.000000000 -0400
+++ linux-2.6.23-rc2/include/asm-i386/atomic.h 2007-08-09 06:47:47.000000000 -0400
@@ -25,7 +25,7 @@ typedef struct { int counter; } atomic_t
*
* Atomically reads the value of @v.
*/
-#define atomic_read(v) ((v)->counter)
+#define atomic_read(v) (*(volatile int *)&(v)->counter)
/**
* atomic_set - set atomic variable
^ permalink raw reply
* [PATCH 7/24] make atomic_read() behave consistently on h8300
From: Chris Snook @ 2007-08-09 13:44 UTC (permalink / raw)
To: linux-kernel, linux-arch, torvalds
Cc: netdev, akpm, ak, heiko.carstens, davem, schwidefsky, wensong,
horms, wjiang, cfriesen, zlynx, rpjday, jesper.juhl
From: Chris Snook <csnook@redhat.com>
Make atomic_read() volatile on h8300. This ensures that busy-waiting
for an interrupt handler to change an atomic_t won't get compiled to an
infinite loop, consistent with SMP architectures.
Signed-off-by: Chris Snook <csnook@redhat.com>
--- linux-2.6.23-rc2-orig/include/asm-h8300/atomic.h 2007-07-08 19:32:17.000000000 -0400
+++ linux-2.6.23-rc2/include/asm-h8300/atomic.h 2007-08-09 06:45:43.000000000 -0400
@@ -9,7 +9,11 @@
typedef struct { int counter; } atomic_t;
#define ATOMIC_INIT(i) { (i) }
-#define atomic_read(v) ((v)->counter)
+/*
+ * Casting to volatile here minimizes the need for barriers,
+ * without having to declare the type itself as volatile.
+ */
+#define atomic_read(v) (*(volatile int *)&(v)->counter)
#define atomic_set(v, i) (((v)->counter) = i)
#include <asm/system.h>
^ permalink raw reply
* [PATCH 9/24] make atomic_read() behave consistently on ia64
From: Chris Snook @ 2007-08-09 13:51 UTC (permalink / raw)
To: linux-kernel, linux-arch, torvalds
Cc: netdev, akpm, ak, heiko.carstens, davem, schwidefsky, wensong,
horms, wjiang, cfriesen, zlynx, rpjday, jesper.juhl
From: Chris Snook <csnook@redhat.com>
Purify volatile use for atomic[64]_t on ia64.
Signed-off-by: Chris Snook <csnook@redhat.com>
--- linux-2.6.23-rc2-orig/include/asm-ia64/atomic.h 2007-07-08 19:32:17.000000000 -0400
+++ linux-2.6.23-rc2/include/asm-ia64/atomic.h 2007-08-09 06:53:48.000000000 -0400
@@ -17,18 +17,18 @@
#include <asm/intrinsics.h>
#include <asm/system.h>
-/*
- * On IA-64, counter must always be volatile to ensure that that the
- * memory accesses are ordered.
- */
-typedef struct { volatile __s32 counter; } atomic_t;
-typedef struct { volatile __s64 counter; } atomic64_t;
+typedef struct { __s32 counter; } atomic_t;
+typedef struct { __s64 counter; } atomic64_t;
#define ATOMIC_INIT(i) ((atomic_t) { (i) })
#define ATOMIC64_INIT(i) ((atomic64_t) { (i) })
-#define atomic_read(v) ((v)->counter)
-#define atomic64_read(v) ((v)->counter)
+/*
+ * Casting to volatile here minimizes the need for barriers,
+ * without having to declare the type itself as volatile.
+ */
+#define atomic_read(v) (*(volatile __s32 *)&(v)->counter)
+#define atomic64_read(v) (*(volatile __s64 *)&(v)->counter)
#define atomic_set(v,i) (((v)->counter) = (i))
#define atomic64_set(v,i) (((v)->counter) = (i))
^ permalink raw reply
* 2.6.23-rc2-mm1: e1000e global symbols must be renamed
From: Adrian Bunk @ 2007-08-09 13:51 UTC (permalink / raw)
To: Andrew Morton, e1000-devel, jgarzik; +Cc: linux-kernel, netdev
In-Reply-To: <20070809015106.cd0bfc53.akpm@linux-foundation.org>
On Thu, Aug 09, 2007 at 01:51:06AM -0700, Andrew Morton wrote:
>...
> - There is a new e1000 driver in git-netdev-all, called e1000e. I'm sure
> the developers would like it tested. Please cc netdev@vger.kernel.org on
> any reports.
>...
> Changes since 2.6.23-rc2-mm1:
>...
> git-netdev-all.patch
>...
> git trees
>...
<-- snip -->
...
LD drivers/net/built-in.o
drivers/net/e1000e/built-in.o: In function `e1000_read_mac_addr':
(.text+0x3470): multiple definition of `e1000_read_mac_addr'
drivers/net/e1000/built-in.o:(.text+0xb6cc): first defined here
drivers/net/e1000e/built-in.o: In function `e1000_set_ethtool_ops':
(.text+0x594d): multiple definition of `e1000_set_ethtool_ops'
drivers/net/e1000/built-in.o:(.text+0xc97a): first defined here
...
make[3]: *** [drivers/net/built-in.o] Error 1
<-- snip -->
cu
Adrian
--
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed
^ permalink raw reply
* [PATCH 10/24] make atomic_read() behave consistently on m32r
From: Chris Snook @ 2007-08-09 13:53 UTC (permalink / raw)
To: linux-kernel, linux-arch, torvalds
Cc: netdev, akpm, ak, heiko.carstens, davem, schwidefsky, wensong,
horms, wjiang, cfriesen, zlynx, rpjday, jesper.juhl
From: Chris Snook <csnook@redhat.com>
Purify volatile use for atomic_t on m32r.
Signed-off-by: Chris Snook <csnook@redhat.com>
--- linux-2.6.23-rc2-orig/include/asm-m32r/atomic.h 2007-07-08 19:32:17.000000000 -0400
+++ linux-2.6.23-rc2/include/asm-m32r/atomic.h 2007-08-09 06:55:53.000000000 -0400
@@ -22,7 +22,7 @@
* on us. We need to use _exactly_ the address the user gave us,
* not some alias that contains the same information.
*/
-typedef struct { volatile int counter; } atomic_t;
+typedef struct { int counter; } atomic_t;
#define ATOMIC_INIT(i) { (i) }
@@ -32,7 +32,7 @@ typedef struct { volatile int counter; }
*
* Atomically reads the value of @v.
*/
-#define atomic_read(v) ((v)->counter)
+#define atomic_read(v) (*(volatile int *)&(v)->counter)
/**
* atomic_set - set atomic variable
^ permalink raw reply
* Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures
From: Arnd Bergmann @ 2007-08-09 12:41 UTC (permalink / raw)
To: Chris Snook
Cc: linux-kernel, linux-arch, torvalds, netdev, akpm, ak,
heiko.carstens, davem, schwidefsky, wensong, horms, wjiang,
cfriesen, zlynx, rpjday, jesper.juhl
In-Reply-To: <20070809131423.GA9927@shell.boston.redhat.com>
On Thursday 09 August 2007, Chris Snook wrote:
> This patchset makes the behavior of atomic_read uniform by removing the
> volatile keyword from all atomic_t and atomic64_t definitions that currently
> have it, and instead explicitly casts the variable as volatile in
> atomic_read(). This leaves little room for creative optimization by the
> compiler, and is in keeping with the principles behind "volatile considered
> harmful".
>
Just an idea: since all architectures already include asm-generic/atomic.h,
why not move the definitions of atomic_t and atomic64_t, as well as anything
that does not involve architecture specific inline assembly into the generic
header?
Arnd <><
^ permalink raw reply
* [PATCH 11/24] make atomic_read() behave consistently on m68knommu
From: Chris Snook @ 2007-08-09 13:56 UTC (permalink / raw)
To: linux-kernel, linux-arch, torvalds
Cc: netdev, akpm, ak, heiko.carstens, davem, schwidefsky, wensong,
horms, wjiang, cfriesen, zlynx, rpjday, jesper.juhl
From: Chris Snook <csnook@redhat.com>
Make atomic_read() volatile on m68knommu. This ensures that busy-waiting
for an interrupt handler to change an atomic_t won't get compiled to an
infinite loop, consistent with SMP architectures.
Signed-off-by: Chris Snook <csnook@redhat.com>
--- linux-2.6.23-rc2-orig/include/asm-m68knommu/atomic.h 2007-07-08 19:32:17.000000000 -0400
+++ linux-2.6.23-rc2/include/asm-m68knommu/atomic.h 2007-08-09 07:00:24.000000000 -0400
@@ -15,7 +15,11 @@
typedef struct { int counter; } atomic_t;
#define ATOMIC_INIT(i) { (i) }
-#define atomic_read(v) ((v)->counter)
+/*
+ * Casting to volatile here minimizes the need for barriers,
+ * without having to declare the type itself as volatile.
+ */
+#define atomic_read(v) (*(volatile int *)&(v)->counter)
#define atomic_set(v, i) (((v)->counter) = i)
static __inline__ void atomic_add(int i, atomic_t *v)
^ permalink raw reply
* [PATCH 12/24] make atomic_read() behave consistently on m68k
From: Chris Snook @ 2007-08-09 13:58 UTC (permalink / raw)
To: linux-kernel, linux-arch, torvalds
Cc: netdev, akpm, ak, heiko.carstens, davem, schwidefsky, wensong,
horms, wjiang, cfriesen, zlynx, rpjday, jesper.juhl
From: Chris Snook <csnook@redhat.com>
Make atomic_read() volatile on m68k. This ensures that busy-waiting
for an interrupt handler to change an atomic_t won't get compiled to an
infinite loop, consistent with SMP architectures.
Signed-off-by: Chris Snook <csnook@redhat.com>
--- linux-2.6.23-rc2-orig/include/asm-m68k/atomic.h 2007-07-08 19:32:17.000000000 -0400
+++ linux-2.6.23-rc2/include/asm-m68k/atomic.h 2007-08-09 06:58:24.000000000 -0400
@@ -16,7 +16,11 @@
typedef struct { int counter; } atomic_t;
#define ATOMIC_INIT(i) { (i) }
-#define atomic_read(v) ((v)->counter)
+/*
+ * Casting to volatile here minimizes the need for barriers,
+ * without having to declare the type itself as volatile.
+ */
+#define atomic_read(v) (*(volatile int *)&(v)->counter)
#define atomic_set(v, i) (((v)->counter) = i)
static inline void atomic_add(int i, atomic_t *v)
^ permalink raw reply
* [PATCH 13/24] make atomic_read() behave consistently on mips
From: Chris Snook @ 2007-08-09 14:00 UTC (permalink / raw)
To: linux-kernel, linux-arch, torvalds
Cc: netdev, akpm, ak, heiko.carstens, davem, schwidefsky, wensong,
horms, wjiang, cfriesen, zlynx, rpjday, jesper.juhl
From: Chris Snook <csnook@redhat.com>
Purify volatile use for atomic[64]_t on mips.
Signed-off-by: Chris Snook <csnook@redhat.com>
--- linux-2.6.23-rc2-orig/include/asm-mips/atomic.h 2007-08-08 17:48:53.000000000 -0400
+++ linux-2.6.23-rc2/include/asm-mips/atomic.h 2007-08-09 07:02:50.000000000 -0400
@@ -20,7 +20,7 @@
#include <asm/war.h>
#include <asm/system.h>
-typedef struct { volatile int counter; } atomic_t;
+typedef struct { int counter; } atomic_t;
#define ATOMIC_INIT(i) { (i) }
@@ -30,7 +30,7 @@ typedef struct { volatile int counter; }
*
* Atomically reads the value of @v.
*/
-#define atomic_read(v) ((v)->counter)
+#define atomic_read(v) (*(volatile int *)&(v)->counter)
/*
* atomic_set - set atomic variable
@@ -404,7 +404,7 @@ static __inline__ int atomic_add_unless(
#ifdef CONFIG_64BIT
-typedef struct { volatile long counter; } atomic64_t;
+typedef struct { long counter; } atomic64_t;
#define ATOMIC64_INIT(i) { (i) }
@@ -413,7 +413,7 @@ typedef struct { volatile long counter;
* @v: pointer of type atomic64_t
*
*/
-#define atomic64_read(v) ((v)->counter)
+#define atomic64_read(v) (*(volatile long *)&(v)->counter)
/*
* atomic64_set - set atomic variable
^ permalink raw reply
* [PATCH 14/24] make atomic_read() behave consistently on parisc
From: Chris Snook @ 2007-08-09 14:01 UTC (permalink / raw)
To: linux-kernel, linux-arch, torvalds
Cc: netdev, akpm, ak, heiko.carstens, davem, schwidefsky, wensong,
horms, wjiang, cfriesen, zlynx, rpjday, jesper.juhl
From: Chris Snook <csnook@redhat.com>
Purify volatile use for atomic[64]_t on parisc.
Signed-off-by: Chris Snook <csnook@redhat.com>
--- linux-2.6.23-rc2-orig/include/asm-parisc/atomic.h 2007-07-08 19:32:17.000000000 -0400
+++ linux-2.6.23-rc2/include/asm-parisc/atomic.h 2007-08-09 07:11:38.000000000 -0400
@@ -128,7 +128,7 @@ __cmpxchg(volatile void *ptr, unsigned l
* Cache-line alignment would conflict with, for example, linux/module.h
*/
-typedef struct { volatile int counter; } atomic_t;
+typedef struct { int counter; } atomic_t;
/* It's possible to reduce all atomic operations to either
* __atomic_add_return, atomic_set and atomic_read (the latter
@@ -157,9 +157,13 @@ static __inline__ void atomic_set(atomic
_atomic_spin_unlock_irqrestore(v, flags);
}
+/*
+ * Casting to volatile here minimizes the need for barriers,
+ * without having to declare the type itself as volatile.
+ */
static __inline__ int atomic_read(const atomic_t *v)
{
- return v->counter;
+ return (*(volatile int *)&(v)->counter);
}
/* exported interface */
@@ -227,7 +231,7 @@ static __inline__ int atomic_add_unless(
#ifdef CONFIG_64BIT
-typedef struct { volatile s64 counter; } atomic64_t;
+typedef struct { s64 counter; } atomic64_t;
#define ATOMIC64_INIT(i) ((atomic64_t) { (i) })
@@ -258,7 +262,7 @@ atomic64_set(atomic64_t *v, s64 i)
static __inline__ s64
atomic64_read(const atomic64_t *v)
{
- return v->counter;
+ return (*(volatile s64 *)&(v)->counter);
}
#define atomic64_add(i,v) ((void)(__atomic64_add_return( ((s64)i),(v))))
^ permalink raw reply
* [PATCH 15/24] make atomic_read() behave consistently on powerpc
From: Chris Snook @ 2007-08-09 14:04 UTC (permalink / raw)
To: linux-kernel, linux-arch, torvalds
Cc: netdev, akpm, ak, heiko.carstens, davem, schwidefsky, wensong,
horms, wjiang, cfriesen, zlynx, rpjday, jesper.juhl
From: Chris Snook <csnook@redhat.com>
Purify volatile use for atomic[64]_t on powerpc.
Signed-off-by: Chris Snook <csnook@redhat.com>
--- linux-2.6.23-rc2-orig/include/asm-powerpc/atomic.h 2007-07-08 19:32:17.000000000 -0400
+++ linux-2.6.23-rc2/include/asm-powerpc/atomic.h 2007-08-09 07:15:21.000000000 -0400
@@ -5,7 +5,7 @@
* PowerPC atomic operations
*/
-typedef struct { volatile int counter; } atomic_t;
+typedef struct { int counter; } atomic_t;
#ifdef __KERNEL__
#include <linux/compiler.h>
@@ -15,7 +15,11 @@ typedef struct { volatile int counter; }
#define ATOMIC_INIT(i) { (i) }
-#define atomic_read(v) ((v)->counter)
+/*
+ * Casting to volatile here minimizes the need for barriers,
+ * without having to declare the type itself as volatile.
+ */
+#define atomic_read(v) (*(volatile int *)&(v)->counter)
#define atomic_set(v,i) (((v)->counter) = (i))
static __inline__ void atomic_add(int a, atomic_t *v)
@@ -240,11 +244,11 @@ static __inline__ int atomic_dec_if_posi
#ifdef __powerpc64__
-typedef struct { volatile long counter; } atomic64_t;
+typedef struct { long counter; } atomic64_t;
#define ATOMIC64_INIT(i) { (i) }
-#define atomic64_read(v) ((v)->counter)
+#define atomic64_read(v) (*(volatile long *)&(v)->counter)
#define atomic64_set(v,i) (((v)->counter) = (i))
static __inline__ void atomic64_add(long a, atomic64_t *v)
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox