* Re: [PATCH 2/24] make atomic_read() behave consistently on arm
From: Russell King @ 2007-08-09 16:06 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: <20070809133028.GA13241@shell.boston.redhat.com>
On Thu, Aug 09, 2007 at 09:30:28AM -0400, Chris Snook wrote:
> From: Chris Snook <csnook@redhat.com>
>
> Purify volatile use for atomic_t on arm.
>
> Signed-off-by: Chris Snook <csnook@redhat.com>
Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
^ permalink raw reply
* Re: [PATCH 24/24] document volatile atomic_read() behavior
From: Segher Boessenkool @ 2007-08-09 15:59 UTC (permalink / raw)
To: Chris Snook
Cc: wjiang, wensong, heiko.carstens, linux-kernel, ak, cfriesen,
netdev, horms, akpm, linux-arch, torvalds, schwidefsky, davem,
zlynx, rpjday, jesper.juhl
In-Reply-To: <20070809142430.GA19817@shell.boston.redhat.com>
> Historically this has been
> +accomplished by declaring the counter itself to be volatile, but the
> +ambiguity of the C standard on the semantics of volatile make this
> practice
> +vulnerable to overly creative interpretation by compilers.
It's even worse when accessing through a volatile casted pointer;
see for example the recent(*) GCC bugs in that area.
(*) Well, not _all_ that recent. No one should be using the 3.x
series anymore, right?
> Explicit
> +casting in atomic_read() ensures consistent behavior across
> architectures
> +and compilers.
Even modulo compiler bugs, what makes you believe that?
Segher
^ permalink raw reply
* Re: [PATCH 1/24] make atomic_read() behave consistently on alpha
From: Segher Boessenkool @ 2007-08-09 15:50 UTC (permalink / raw)
To: Chris Snook
Cc: wjiang, rpjday, wensong, heiko.carstens, linux-kernel, ak, netdev,
paulmck, horms, akpm, linux-arch, jesper.juhl, torvalds,
schwidefsky, davem, cfriesen, zlynx
In-Reply-To: <46BB31A6.4080507@redhat.com>
> We can't have split stores because we don't use atomic64_t on 32-bit
> architectures.
That's not true; the compiler is free to split all stores
(and reads) from memory however it wants. It is debatable
whether "volatile" would prevent this as well, certainly
it is unsafe if you want to be portable. GCC will do its
best to not split volatile memory accesses, but bugs in
this area do happen a lot (because the compiler code for
volatile isn't as well exercised as most other compiler
code, and because it is simply a hard subject; and there
is no real formalised model for what GCC should do).
The only safe way to get atomic accesses is to write
assembler code. Are there any downsides to that? I don't
see any.
Segher
^ permalink raw reply
* Re: 2.6.23-rc2: WARNING: at kernel/irq/resend.c:70 check_irq_resend()
From: Jarek Poplawski @ 2007-08-09 15:54 UTC (permalink / raw)
To: John Stoffel
Cc: linux-kernel, shemminger, vignaud, marcin.slusarz, tglx, mingo,
torvalds, akpm, alan, linux-net, netdev
In-Reply-To: <18107.11431.838905.331157@stoffel.org>
On Thu, Aug 09, 2007 at 11:03:03AM -0400, John Stoffel wrote:
>
> Hi,
Hi, read below, please...
>
> I'm opening this ticket as a new subject, even though it looks like it
> might be related to the thread "Networking dies after random time".
> Sorry for the wide CC list, but since my network hasn't died since I
> rebooted into 2.6.23-rc2 (after 30+ days at 2.6.22-rc7), I'm wondering
> if the problem is more than networking related.
>
> Honestly, I haven't gone back over the previous thread in detail, so I
> might be missing info here.
>
> System details: Dell Precision 610MT, Intel 440GX chipset, Dual PIII
> Xeon, 550Mhz, 2gb RAM (upgraded from 768Mb last night), a mix of IDE,
> SCSI and SATA disks in the system. My poor PCI bus! Just upgraded to
> 2.6.23-rc2. Interrupts looks like this:
>
> > cat /proc/interrupts
> CPU0 CPU1
> 0: 280 1 IO-APIC-edge timer
> 1: 788 0 IO-APIC-edge i8042
> 6: 1 4 IO-APIC-edge floppy
> 8: 0 1 IO-APIC-edge rtc
> 9: 0 0 IO-APIC-fasteoi acpi
> 11: 82410 1239 IO-APIC-edge Cyclom-Y
> 12: 279 106 IO-APIC-edge i8042
> 14: 440901 4266 IO-APIC-edge libata
> 15: 0 0 IO-APIC-edge libata
> 16: 2394727 42983 IO-APIC-fasteoi ohci_hcd:usb3, Ensoniq
> AudioPCI, mga@pci:0000:01:00.0
> 17: 2237362 1110 IO-APIC-fasteoi sata_sil,
> ehci_hcd:usb1, eth0
> 18: 126520 31978 IO-APIC-fasteoi aic7xxx, aic7xxx, ide2,
> ide3, ohci1394
> 19: 0 0 IO-APIC-fasteoi ohci_hcd:usb2,
> uhci_hcd:usb4
> NMI: 0 0
> LOC: 40672484 40672246
> ERR: 0
> MIS: 0
>
> I've only seen the one Warning oops, and backups and other system
> processes have been running for the past 12 hours without a problem.
>
>
> [ 187.747442] Probing IDE interface ide2...
> [ 188.011634] hde: WDC WD1200JB-00CRA1, ATA DISK drive
> [ 188.623038] WARNING: at kernel/irq/resend.c:70 check_irq_resend()
> [ 188.623105] [<c0149e38>] check_irq_resend+0xa8/0xc0
> [ 188.623204] [<c01499d3>] enable_irq+0xc3/0xd0
> [ 188.623295] [<f8867280>] probe_hwif+0x670/0x7c0 [ide_core]
> [ 188.623448] [<f8869f04>] do_ide_setup_pci_device+0x154/0x480
> [ide_core]
> [ 188.623571] [<f8867d6c>] probe_hwif_init_with_fixup+0xc/0x90
> [ide_core]
> [ 188.623690] [<f88817d0>] init_setup_hpt302+0x0/0x30 [hpt366]
> [ 188.623791] [<f886a39b>] ide_setup_pci_device+0x7b/0xc0 [ide_core]
> [ 188.623909] [<f88817d0>] init_setup_hpt302+0x0/0x30 [hpt366]
> [ 188.624004] [<f88811ed>] hpt366_init_one+0x8d/0xa0 [hpt366]
> [ 188.624095] [<f88817d0>] init_setup_hpt302+0x0/0x30 [hpt366]
> [ 188.624187] [<f8881e50>] init_chipset_hpt366+0x0/0x680 [hpt366]
> [ 188.624281] [<f8882680>] init_hwif_hpt366+0x0/0x380 [hpt366]
> [ 188.624372] [<f8881800>] init_dma_hpt366+0x0/0xe0 [hpt366]
> [ 188.624466] [<c0265fc6>] pci_device_probe+0x56/0x80
> [ 188.624565] [<c02d0f8e>] driver_probe_device+0x8e/0x190
> [ 188.624669] [<c02d11fe>] __driver_attach+0x9e/0xa0
> [ 188.624756] [<c02d038a>] bus_for_each_dev+0x3a/0x60
> [ 188.624845] [<c02d0e06>] driver_attach+0x16/0x20
> [ 188.624932] [<c02d1160>] __driver_attach+0x0/0xa0
> [ 188.625017] [<c02d075a>] bus_add_driver+0x8a/0x1b0
> [ 188.625107] [<c0266173>] __pci_register_driver+0x53/0xa0
> [ 188.625197] [<c0144d5d>] sys_init_module+0x13d/0x1820
> [ 188.625315] [<f8844000>] snd_timer_find+0x0/0x90 [snd_timer]
> [ 188.625424] [<c0149530>] disable_irq+0x0/0x30
> [ 188.625513] [<c0108b7d>] sys_mmap2+0xcd/0xd0
> [ 188.625612] [<c0104266>] syscall_call+0x7/0xb
> [ 188.625701] [<c0410000>] rpc_get_inode+0x0/0x80
> [ 188.625798] =======================
> [ 188.625871] hde: selected mode 0x45
> [ 188.626817] ide2 at 0xecf8-0xecff,0xecf2 on irq 18
> [ 188.627080] Probing IDE interface ide3...
> [ 188.891165] hdg: WDC WD1200JB-00EVA0, ATA DISK drive
> [ 189.502580] hdg: selected mode 0x45
> [ 189.503698] ide3 at 0xece0-0xece7,0xecda on irq 18
>
>
> Let
I'm not sure I don't miss anything (a little in hurry now), but this
warning's aim was purely diagnostical and nothing wrong is meant!
Unless there is something wrong... Then please try to be more explicit.
If you prefer to not see this, there is my patch proposal somewhere
in this older thread:
Subject: [patch] genirq: temporary fix for level-triggered IRQ resend
Date: Wed, 8 Aug 2007 13:00:37 +0200
On the other hand, if it works OK, it would be better to let it be
tested more like this...
Regards,
Jarek P.
^ permalink raw reply
* Re: [PATCH 1/24] make atomic_read() behave consistently on alpha
From: Chris Snook @ 2007-08-09 15:24 UTC (permalink / raw)
To: paulmck
Cc: linux-kernel, linux-arch, torvalds, netdev, akpm, ak,
heiko.carstens, davem, schwidefsky, wensong, horms, wjiang,
cfriesen, zlynx, rpjday, jesper.juhl
In-Reply-To: <20070809150445.GB8424@linux.vnet.ibm.com>
Paul E. McKenney wrote:
> On Thu, Aug 09, 2007 at 10:53:14AM -0400, Chris Snook wrote:
>> Paul E. McKenney wrote:
>>> Why not the same access-once semantics for atomic_set() as
>>> for atomic_read()? As this patch stands, it might introduce
>>> architecture-specific compiler-induced bugs due to the fact that
>>> atomic_set() used to imply volatile behavior but no longer does.
>> When we make the volatile cast in atomic_read(), we're casting an rvalue to
>> volatile. This unambiguously tells the compiler that we want to re-load
>> that register from memory. What's "volatile behavior" for an lvalue?
>
> I was absolutely -not- suggesting volatile behavior for lvalues.
>
> Instead, I am asking for volatile behavior from an -rvalue-. In the
> case of atomic_read(), it is the atomic_t being read from. In the case
> of atomic_set(), it is the atomic_t being written to. As suggested in
> my previous email:
>
> #define atomic_set(v,i) ((*(volatile int *)&(v)->counter) = (i))
> #define atomic64_set(v,i) ((*(volatile long *)&(v)->counter) = (i))
That looks like a volatile lvalue to me. I confess I didn't exactly ace
compilers. Care to explain this?
> Again, the architectures that used to have their "counter" declared
> as volatile will lose volatile semantics on atomic_set() with your
> patch, which might result in bugs due to overly imaginative compiler
> optimizations. The above would prevent any such bugs from appearing.
>
>> A
>> write to an lvalue already implies an eventual write to memory, so this
>> would be a no-op. Maybe you'll write to the register a few times before
>> flushing it to memory, but it will happen eventually. With an rvalue,
>> there's no guarantee that it will *ever* load from memory, which is what
>> volatile fixes.
>>
>> I think what you have in mind is LOCK_PREFIX behavior, which is not the
>> purpose of atomic_set. We use LOCK_PREFIX in the inline assembly for the
>> atomic_* operations that read, modify, and write a value, only because it
>> is necessary to perform that entire transaction atomically.
>
> No LOCK_PREFIX, thank you!!! I just want to make sure that the compiler
> doesn't push the store down out of a loop, split the store, allow the
> store to happen twice (e.g., to allow different code paths to be merged),
> and all the other tricks that the C standard permits compilers to pull.
We can't have split stores because we don't use atomic64_t on 32-bit
architectures. volatile won't save you from pushing stores out of loops unless
you're also doing reads. This is why we use reads to flush writes to mmio
registers. In this case, an atomic_read() with volatile in it will suffice.
Storing twice is perfectly legal here, though it's unlikely that an optimizing
compiler smart enough to create the problem we're addressing would ever do that.
-- Chris
^ permalink raw reply
* Re: [RFC] Re: 2.6.20->2.6.21 - networking dies after random time
From: Jarek Poplawski @ 2007-08-09 15:30 UTC (permalink / raw)
To: Andi Kleen
Cc: Ingo Molnar, Chuck Ebbert, =?iso-8859-2?q? Marcin Ślusarz?=,
Thomas Gleixner, Linus Torvalds, Jean-Baptiste Vignaud,
linux-kernel, shemminger, linux-net, netdev, Andrew Morton,
Alan Cox
In-Reply-To: <p738x8kg0dp.fsf@bingen.suse.de>
On Thu, Aug 09, 2007 at 06:04:34PM +0200, Andi Kleen wrote:
> Jarek Poplawski <jarkao2@o2.pl> writes:
>
> > It seems, we can start to think about some preferred solutions,
> > already. Here are some of my preliminary conclusions and suggestions.
> >
> > The problem of timeouts with some 'older' network cards seems to hit
> > mainly x86_64 arch, and after diagnosing and testing (still beeing
> > done) it's caused by resending level type irqs.
>
> i386 interrupt code should be similar, except for the lack of
> per CPU irqs, but that shouldnt' affect resending.
>
> >
> > Possible solutions:
>
> We should probably at least add some statistic counters to the
> standard kernel to try to detect these cases.
>
> > It looks like these changes are needed for this x86_64 only,
>
> Why?
Maybe I missed something, but considering the popularity of i386
there was not so much of consistent reporting?! I was very surprised,
when I read a few days ago that Linus seems to think that this one
here is only an individual problem...
Jarek P.
^ permalink raw reply
* Re: [PATCH 1/1] NFS: change the ip_map cache code to handle IPv6 addresses
From: Chuck Lever @ 2007-08-09 15:14 UTC (permalink / raw)
To: Aurélien Charbon; +Cc: Mailing list NFSv4, netdev ML, Neil Brown
In-Reply-To: <46BB2E01.2010406@ext.bull.net>
[-- Attachment #1: Type: text/plain, Size: 999 bytes --]
Aurélien Charbon wrote:
>>> @@ -112,12 +112,16 @@
>>> return (hash ^ (hash>>8)) & 0xff;
>>> }
>>> #endif
>>> +static inline int hash_ip6(struct in6_addr ip)
>>> +{
>>> + return (hash_ip(ip.s6_addr32[0]) ^ hash_ip(ip.s6_addr32[1])
>>> ^ hash_ip(ip.s6_addr32[2]) ^ hash_ip(ip.s6_addr32[3])) ;
>>> +}
>>
>>
>> How have you tested the effectiveness of the new hash function?
>
> I have not tested that point but I can easily imagine there are better
> solutions.
> Perhaps we can keep the same function for an IPv4 address (only taking
> the 32 bits of IPv4 addr), and then design one for IPv6 addresses.
I see that, to generate the hash, you would be xor-ing the FF and 00
bytes in the canonicalized IPv4 address. Yes, perhaps a better function
is needed, or as you say, one specifically for IPv6 and one for
canonicalized IPv4.
> Do you have any suggestion on that ?
I don't have anything specific, but you may find something useful if you
poke around elsewhere under net/.
[-- Attachment #2: chuck.lever.vcf --]
[-- Type: text/x-vcard, Size: 302 bytes --]
begin:vcard
fn:Chuck Lever
n:Lever;Chuck
org:Oracle Corporation;Corporate Architecture: Linux Projects Group
adr:;;1015 Granger Avenue;Ann Arbor;MI;48104;USA
title:Principal Member of Staff
tel;work:+1 248 614 5091
x-mozilla-html:FALSE
url:http://oss.oracle.com/~cel
version:2.1
end:vcard
^ permalink raw reply
* Re: [PATCH 1/1] NFS: change the ip_map cache code to handle IPv6 addresses
From: Aurélien Charbon @ 2007-08-09 15:08 UTC (permalink / raw)
To: chuck.lever; +Cc: Mailing list NFSv4, netdev ML, Neil Brown
In-Reply-To: <46BB05B6.5080301@oracle.com>
Thanks for these comments.
Chuck Lever wrote:
> Some naive questions:
>
> 1. If IPv6 support is not configured into the kernel, how does an
> IPv6-only cache work?
Yes it should work.
The INET6 structures are only used as containers for both type of address.
I have tested by unabling IPv6 options in the .config file.
>
> 2. I seem to recall (only quite vaguely) that at some point the
> server might need to use one of the stored addresses to, say, open a
> socket to the client? In that case, on a system with NICs configured
> only with IPv4, is the cached IPv6 address properly converted back to
> IPv4 somehow?
Yes. we can do that by testing the format of the address, as we know
that an IPv4 address is mapped like that:
[00000000] [00000000] [0000FFFF] <32 bits of IPv4 addr>
> Can the cache code tell the difference between a cached IPv6 address
> that was converted from IPv4 and one that was added to the cache as
> IPv6? Sorry I can't remember more clearly.
Yes. If the address starts with ::FFFF, we can infer that is a mapped
IPv4 address.
>
> 3. Would it be better to make the m_addr field a struct sockaddr,
> store a whole address (with address family), and switch on the
> sa_family field?
That is possible but we're able to retrieve the family of the address by
testing its format (IPv4 mapped or not).
But I don't know if it is a better solution.
>
>
>> diff -u -r -N linux-2.6.23-rc1/fs/nfsd/export.c
>> linux-2.6.23-rc1-IPv6-ip_map/fs/nfsd/export.c
>> --- linux-2.6.23-rc1/fs/nfsd/export.c 2007-08-08
>> 17:52:58.000000000 +0200
>> +++ linux-2.6.23-rc1-IPv6-ip_map/fs/nfsd/export.c 2007-08-08
>> 17:49:09.000000000 +0200
>> @@ -1558,6 +1558,7 @@
>> {
>> struct auth_domain *dom;
>> int i, err;
>> + struct in6_addr addr6;
>>
>> /* First, consistency check. */
>> err = -EINVAL;
>> @@ -1576,9 +1577,14 @@
>> goto out_unlock;
>>
>> /* Insert client into hashtable. */
>> - for (i = 0; i < ncp->cl_naddr; i++)
>> - auth_unix_add_addr(ncp->cl_addrlist[i], dom);
>> -
>> + for (i = 0; i < ncp->cl_naddr; i++) {
>> + /* Mapping address */
>> + addr6.s6_addr32[0] = 0;
>> + addr6.s6_addr32[1] = 0;
>> + addr6.s6_addr32[2] = htonl(0xffff);
>> + addr6.s6_addr32[3] = (uint32_t)ncp->cl_addrlist[i].s_addr;
>> + auth_unix_add_addr(addr6, dom);
>> + }
>> auth_unix_forget_old(dom);
>> auth_domain_put(dom);
>
>
> This converts IPv4 addresses to canonical IPv6 as it stores them.
> What happens if a full-blown IPv6 address is encountered? Likewise,
> in nfsctl.c?
I have done this kind of convertion only for compilation an testing with
only IPv4... For the moment.
>
>> @@ -112,12 +112,16 @@
>> return (hash ^ (hash>>8)) & 0xff;
>> }
>> #endif
>> +static inline int hash_ip6(struct in6_addr ip)
>> +{
>> + return (hash_ip(ip.s6_addr32[0]) ^ hash_ip(ip.s6_addr32[1])
>> ^ hash_ip(ip.s6_addr32[2]) ^ hash_ip(ip.s6_addr32[3])) ;
>> +}
>
>
> How have you tested the effectiveness of the new hash function?
I have not tested that point but I can easily imagine there are better
solutions.
Perhaps we can keep the same function for an IPv4 address (only taking
the 32 bits of IPv4 addr), and then design one for IPv6 addresses.
Do you have any suggestion on that ?
>> - seq_printf(m, "%s %d.%d.%d.%d %s\n",
>> + seq_printf(m, "%s %04x.%04x.%04x.%04x.%04x.%04x.%04x.%04x %s\n",
>> im->m_class,
>> - ntohl(addr.s_addr) >> 24 & 0xff,
>> - ntohl(addr.s_addr) >> 16 & 0xff,
>> - ntohl(addr.s_addr) >> 8 & 0xff,
>> - ntohl(addr.s_addr) >> 0 & 0xff,
>> + ntohl(addr.s6_addr32[3]) >> 16 & 0xffff,
>> + ntohl(addr.s6_addr32[3]) & 0xffff,
>> + ntohl(addr.s6_addr32[2]) >> 16 & 0xffff,
>> + ntohl(addr.s6_addr32[2]) & 0xffff,
>> + ntohl(addr.s6_addr32[1]) >> 16 & 0xffff,
>> + ntohl(addr.s6_addr32[1]) & 0xffff,
>> + ntohl(addr.s6_addr32[0]) >> 16 & 0xffff,
>> + ntohl(addr.s6_addr32[0]) & 0xffff,
>> dom
>> );
>> return 0;
>
>
> And I think here NIP6_FMT should be used, but you're not using colons
> between the hex digits. Was that intentional
> ?
No that's a mistake. Colons have to be used here.
Aurélien
--
********************************
Aurelien Charbon
Linux NFSv4 team
Bull SAS
Echirolles - France
http://nfsv4.bullopensource.org/
********************************
^ permalink raw reply
* Re: [PATCH 1/24] make atomic_read() behave consistently on alpha
From: Paul E. McKenney @ 2007-08-09 15:04 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: <46BB2A5A.5090006@redhat.com>
On Thu, Aug 09, 2007 at 10:53:14AM -0400, Chris Snook wrote:
> Paul E. McKenney wrote:
> >Why not the same access-once semantics for atomic_set() as
> >for atomic_read()? As this patch stands, it might introduce
> >architecture-specific compiler-induced bugs due to the fact that
> >atomic_set() used to imply volatile behavior but no longer does.
>
> When we make the volatile cast in atomic_read(), we're casting an rvalue to
> volatile. This unambiguously tells the compiler that we want to re-load
> that register from memory. What's "volatile behavior" for an lvalue?
I was absolutely -not- suggesting volatile behavior for lvalues.
Instead, I am asking for volatile behavior from an -rvalue-. In the
case of atomic_read(), it is the atomic_t being read from. In the case
of atomic_set(), it is the atomic_t being written to. As suggested in
my previous email:
#define atomic_set(v,i) ((*(volatile int *)&(v)->counter) = (i))
#define atomic64_set(v,i) ((*(volatile long *)&(v)->counter) = (i))
Again, the architectures that used to have their "counter" declared
as volatile will lose volatile semantics on atomic_set() with your
patch, which might result in bugs due to overly imaginative compiler
optimizations. The above would prevent any such bugs from appearing.
> A
> write to an lvalue already implies an eventual write to memory, so this
> would be a no-op. Maybe you'll write to the register a few times before
> flushing it to memory, but it will happen eventually. With an rvalue,
> there's no guarantee that it will *ever* load from memory, which is what
> volatile fixes.
>
> I think what you have in mind is LOCK_PREFIX behavior, which is not the
> purpose of atomic_set. We use LOCK_PREFIX in the inline assembly for the
> atomic_* operations that read, modify, and write a value, only because it
> is necessary to perform that entire transaction atomically.
No LOCK_PREFIX, thank you!!! I just want to make sure that the compiler
doesn't push the store down out of a loop, split the store, allow the
store to happen twice (e.g., to allow different code paths to be merged),
and all the other tricks that the C standard permits compilers to pull.
Thanx, Paul
^ permalink raw reply
* 2.6.23-rc2: WARNING: at kernel/irq/resend.c:70 check_irq_resend()
From: John Stoffel @ 2007-08-09 15:03 UTC (permalink / raw)
To: linux-kernel
Cc: jarkao2, shemminger, vignaud, marcin.slusarz, tglx, mingo,
torvalds, akpm, alan, linux-net, netdev
Hi,
I'm opening this ticket as a new subject, even though it looks like it
might be related to the thread "Networking dies after random time".
Sorry for the wide CC list, but since my network hasn't died since I
rebooted into 2.6.23-rc2 (after 30+ days at 2.6.22-rc7), I'm wondering
if the problem is more than networking related.
Honestly, I haven't gone back over the previous thread in detail, so I
might be missing info here.
System details: Dell Precision 610MT, Intel 440GX chipset, Dual PIII
Xeon, 550Mhz, 2gb RAM (upgraded from 768Mb last night), a mix of IDE,
SCSI and SATA disks in the system. My poor PCI bus! Just upgraded to
2.6.23-rc2. Interrupts looks like this:
> cat /proc/interrupts
CPU0 CPU1
0: 280 1 IO-APIC-edge timer
1: 788 0 IO-APIC-edge i8042
6: 1 4 IO-APIC-edge floppy
8: 0 1 IO-APIC-edge rtc
9: 0 0 IO-APIC-fasteoi acpi
11: 82410 1239 IO-APIC-edge Cyclom-Y
12: 279 106 IO-APIC-edge i8042
14: 440901 4266 IO-APIC-edge libata
15: 0 0 IO-APIC-edge libata
16: 2394727 42983 IO-APIC-fasteoi ohci_hcd:usb3, Ensoniq
AudioPCI, mga@pci:0000:01:00.0
17: 2237362 1110 IO-APIC-fasteoi sata_sil,
ehci_hcd:usb1, eth0
18: 126520 31978 IO-APIC-fasteoi aic7xxx, aic7xxx, ide2,
ide3, ohci1394
19: 0 0 IO-APIC-fasteoi ohci_hcd:usb2,
uhci_hcd:usb4
NMI: 0 0
LOC: 40672484 40672246
ERR: 0
MIS: 0
I've only seen the one Warning oops, and backups and other system
processes have been running for the past 12 hours without a problem.
[ 187.747442] Probing IDE interface ide2...
[ 188.011634] hde: WDC WD1200JB-00CRA1, ATA DISK drive
[ 188.623038] WARNING: at kernel/irq/resend.c:70 check_irq_resend()
[ 188.623105] [<c0149e38>] check_irq_resend+0xa8/0xc0
[ 188.623204] [<c01499d3>] enable_irq+0xc3/0xd0
[ 188.623295] [<f8867280>] probe_hwif+0x670/0x7c0 [ide_core]
[ 188.623448] [<f8869f04>] do_ide_setup_pci_device+0x154/0x480
[ide_core]
[ 188.623571] [<f8867d6c>] probe_hwif_init_with_fixup+0xc/0x90
[ide_core]
[ 188.623690] [<f88817d0>] init_setup_hpt302+0x0/0x30 [hpt366]
[ 188.623791] [<f886a39b>] ide_setup_pci_device+0x7b/0xc0 [ide_core]
[ 188.623909] [<f88817d0>] init_setup_hpt302+0x0/0x30 [hpt366]
[ 188.624004] [<f88811ed>] hpt366_init_one+0x8d/0xa0 [hpt366]
[ 188.624095] [<f88817d0>] init_setup_hpt302+0x0/0x30 [hpt366]
[ 188.624187] [<f8881e50>] init_chipset_hpt366+0x0/0x680 [hpt366]
[ 188.624281] [<f8882680>] init_hwif_hpt366+0x0/0x380 [hpt366]
[ 188.624372] [<f8881800>] init_dma_hpt366+0x0/0xe0 [hpt366]
[ 188.624466] [<c0265fc6>] pci_device_probe+0x56/0x80
[ 188.624565] [<c02d0f8e>] driver_probe_device+0x8e/0x190
[ 188.624669] [<c02d11fe>] __driver_attach+0x9e/0xa0
[ 188.624756] [<c02d038a>] bus_for_each_dev+0x3a/0x60
[ 188.624845] [<c02d0e06>] driver_attach+0x16/0x20
[ 188.624932] [<c02d1160>] __driver_attach+0x0/0xa0
[ 188.625017] [<c02d075a>] bus_add_driver+0x8a/0x1b0
[ 188.625107] [<c0266173>] __pci_register_driver+0x53/0xa0
[ 188.625197] [<c0144d5d>] sys_init_module+0x13d/0x1820
[ 188.625315] [<f8844000>] snd_timer_find+0x0/0x90 [snd_timer]
[ 188.625424] [<c0149530>] disable_irq+0x0/0x30
[ 188.625513] [<c0108b7d>] sys_mmap2+0xcd/0xd0
[ 188.625612] [<c0104266>] syscall_call+0x7/0xb
[ 188.625701] [<c0410000>] rpc_get_inode+0x0/0x80
[ 188.625798] =======================
[ 188.625871] hde: selected mode 0x45
[ 188.626817] ide2 at 0xecf8-0xecff,0xecf2 on irq 18
[ 188.627080] Probing IDE interface ide3...
[ 188.891165] hdg: WDC WD1200JB-00EVA0, ATA DISK drive
[ 189.502580] hdg: selected mode 0x45
[ 189.503698] ide3 at 0xece0-0xece7,0xecda on irq 18
Let
^ permalink raw reply
* Re: [PATCH] make atomic_t volatile on all architectures
From: Linus Torvalds @ 2007-08-09 15:00 UTC (permalink / raw)
To: Jerry Jiang
Cc: 7eggert, Chris Snook, akpm, ak, heiko.carstens, davem,
linux-kernel, netdev, schwidefsky, wensong, horms, cfriesen,
zlynx
In-Reply-To: <20070809171832.1568864b.wjiang@resilience.com>
On Thu, 9 Aug 2007, Jerry Jiang wrote:
>
> and still not to said "Why the *volatile-accesses-in-code* is
> acceptable"
I don't think volatile is necessarily wonderful in code _either_. So I
think the "atomic_read()" issue would be even better off if we just made
sure everybody behaves well and has the right barriers.
So the difference between "volatile in code" and "volatile on data
structures" is that the latter is *always* wrong (and wrong for very
fundamental reasons).
But "volatile in code" can be perfectly fine. If you hide the volatile in
an accessor function, and just specify that the rules for that function is
that it always loads from memory but doesn't imply any memory barriers,
than that is fine. And I think those kinds of semantics may well be
perfectly sane for "atomic_read()".
So I think a volatile access inside "atomic_read()" at least has
well-defined semantics. Are they the best possible semantics? I dunno. I
can imagine that it's perfectly fine, but on the other hand, it would be
interesting to see any code for which it matters - it's entirely possible
that the code itself is the problem, and should instead have a
"cpu_relax()" or similar that implies a barrier.
Linus
^ permalink raw reply
* Re: [PATCH 1/24] make atomic_read() behave consistently on alpha
From: Chris Snook @ 2007-08-09 14:53 UTC (permalink / raw)
To: paulmck
Cc: linux-kernel, linux-arch, torvalds, netdev, akpm, ak,
heiko.carstens, davem, schwidefsky, wensong, horms, wjiang,
cfriesen, zlynx, rpjday, jesper.juhl
In-Reply-To: <20070809143255.GA8424@linux.vnet.ibm.com>
Paul E. McKenney wrote:
> Why not the same access-once semantics for atomic_set() as
> for atomic_read()? As this patch stands, it might introduce
> architecture-specific compiler-induced bugs due to the fact that
> atomic_set() used to imply volatile behavior but no longer does.
When we make the volatile cast in atomic_read(), we're casting an rvalue to
volatile. This unambiguously tells the compiler that we want to re-load that
register from memory. What's "volatile behavior" for an lvalue? A write to an
lvalue already implies an eventual write to memory, so this would be a no-op.
Maybe you'll write to the register a few times before flushing it to memory, but
it will happen eventually. With an rvalue, there's no guarantee that it will
*ever* load from memory, which is what volatile fixes.
I think what you have in mind is LOCK_PREFIX behavior, which is not the purpose
of atomic_set. We use LOCK_PREFIX in the inline assembly for the atomic_*
operations that read, modify, and write a value, only because it is necessary to
perform that entire transaction atomically.
-- Chris
^ permalink raw reply
* [RFC] Re: 2.6.20->2.6.21 - networking dies after random time
From: Jarek Poplawski @ 2007-08-09 14:50 UTC (permalink / raw)
To: Ingo Molnar
Cc: Chuck Ebbert, Marcin Ślusarz, Thomas Gleixner,
Linus Torvalds, Jean-Baptiste Vignaud, linux-kernel, shemminger,
linux-net, netdev, Andrew Morton, Alan Cox
In-Reply-To: <20070806190812.GD26868@elte.hu>
It seems, we can start to think about some preferred solutions,
already. Here are some of my preliminary conclusions and suggestions.
The problem of timeouts with some 'older' network cards seems to hit
mainly x86_64 arch, and after diagnosing and testing (still beeing
done) it's caused by resending level type irqs.
Possible solutions:
1. Reverting the whole "do not mask interrupts by default" patch:
Seems to be the most natural, but there are some minuses: this patch
has been here for a long time and some archs/drivers could have been
changed in the meantime and depend on this; this could also remove
possible good sides of this patch for which it was aimed (mainly for
edge type irqs), so some old problems could be back.
2. Excluding all level type irqs from resending. There are 2 ways:
a) Just like this is done now in -rc2 with Thomas' patch and AFAIK
seems to work very well; the way to find the type of interrupt by
the name of a handler looks a bit 'temporary' but it's simple and
working; it could be a bit moved and under #ifdef CONFIG_, so
this could affect only choosen ones.
b) Alternatively this could be done by e.g. assigning separate
irq_chip structures for edge and level handlers, so for levels there
would be chip->retrigger == NULL; so this could be done by arches
without any need for 'global' changes. The only minus here is a few
bytes of memory more; on the other hand it looks more readable and
elastic.
3. Using HARDIRQS_SW_RESEND for level type irqs:
seems to work, but needs more testing; but if #2 is theoretically
and practically OK, why bother. This also doesn't need any global
changes.
I prefer 2b + 3: since this is very delicate measure, I think there
should be visible CONFIG_ option: at least for disabling
chip->retrigger handler if used by arch and maybe for using
_SW_RESEND instead, as well.
It looks like these changes are needed for this x86_64 only, but
in my opinion some (good?!) apics could sometimes work OK with this
level resending only "by chance": theoretically it's questionable:
since edge type irqs used for this should be resent if not acked, and
level handlers ack time depends on how long drivers do their job,
there are possible strange and hard to repeat effects, for no reason
(if these levels can really always be skipped without any problem,
like seen in testing).
Then of course it would be easier to do this 'globally' with 2a.:
skipping by default (but #ifdef) chip->retrigger namely for:
handler_fasteoi_irq and handler_level_irq and handler_simple_irq;
handle_edge_irq plus others are always tried.
I hope, Ingo or Thomas will use something of these, or let us know
about maybe something else which should by tested for final inclusion.
Regards,
Jarek P.
^ permalink raw reply
* Re: net-2.6.24 GIT state
From: Ilpo Järvinen @ 2007-08-09 14:40 UTC (permalink / raw)
To: David Miller; +Cc: Netdev
In-Reply-To: <20070809.041034.93473045.davem@davemloft.net>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1326 bytes --]
On Thu, 9 Aug 2007, David Miller wrote:
> From: "Ilpo_Järvinen" <ilpo.jarvinen@helsinki.fi>
> Date: Thu, 9 Aug 2007 12:50:51 +0300 (EEST)
>
> Ok, in order to get this started I merged the above patches
> into net-2.6.24
>
> I'm exhausted and done for today. :-)
:-)
> We can work on the other bits next.
Next step:
http://www.cs.helsinki.fi/u/ijjarvin/tcp-rebase/after-reorder/
...I put some effort to reorganization and there's the result, please
_don't_ take the new SACK block-validator yet as I haven't yet put it
under any testing (it's the only difference this reordering of patches
seems to have introduced :-)). The first five commits touch left_out (4
latest fix bug and "flaws" I made in the first commit), I could combine
them into a single one if you want to.
...Notice that part of the next step includes work that hasn't yet been
posted to netdev at all (was from my not yet submitted wip portion of
it). Would you prefer me to post them using the usual procedure?
> > ...Taking IsReno/IsFack removal is probably going to be hardest one, as it
> > will conflict a lot. ...Maybe some sed work in patches would reduce # of
> > conflicts instead of trying cherry-pick+rebase.
>
> Yes, such patch hand editing techniques can help in these cases.
...it helped a lot in the process :-).
--
i.
^ permalink raw reply
* Re: [PATCH 1/1] af_packet: don't enable timestamps in mmap'ed sockets
From: Evgeniy Polyakov @ 2007-08-09 14:33 UTC (permalink / raw)
To: Unai Uribarri; +Cc: netdev, linux-kernel
In-Reply-To: <1186669314.24669.56.camel@localhost.localdomain>
On Thu, Aug 09, 2007 at 04:21:54PM +0200, Unai Uribarri (unai.uribarri@optenet.com) wrote:
> The attached patch removes the automatic timestamp activation, that
> only mmap'ed AF_PACKET sockets perform. I known it can break user
> applications, but I believe that it's the correct solution.
How tcpdump with mmap libpcap will work with it?
--
Evgeniy Polyakov
^ permalink raw reply
* Re: [PATCH 16/24] make atomic_read() behave consistently on s390
From: Martin Schwidefsky @ 2007-08-09 14:36 UTC (permalink / raw)
To: Chris Snook
Cc: linux-kernel, linux-arch, torvalds, netdev, akpm, ak,
heiko.carstens, davem, wensong, horms, wjiang, cfriesen, zlynx,
rpjday, jesper.juhl
In-Reply-To: <20070809140655.GA17731@shell.boston.redhat.com>
On Thu, 2007-08-09 at 10:06 -0400, Chris Snook wrote:
> From: Chris Snook <csnook@redhat.com>
>
> Make atomic[64]_read() volatile on s390, to ensure memory is actually read
> each time.
>
> Signed-off-by: Chris Snook <csnook@redhat.com>
Acked-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
--
blue skies,
Martin.
"Reality continues to ruin my life." - Calvin.
^ permalink raw reply
* Re: [PATCH 1/24] make atomic_read() behave consistently on alpha
From: Paul E. McKenney @ 2007-08-09 14:32 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: <20070809132442.GA13042@shell.boston.redhat.com>
On Thu, Aug 09, 2007 at 09:24:42AM -0400, Chris Snook wrote:
> From: Chris Snook <csnook@redhat.com>
>
> Purify volatile use for atomic[64]_t on alpha.
Why not the same access-once semantics for atomic_set() as
for atomic_read()? As this patch stands, it might introduce
architecture-specific compiler-induced bugs due to the fact that
atomic_set() used to imply volatile behavior but no longer does.
See below for one approach to fixing this.
> 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))
How about:
#define atomic_set(v,i) ((*(volatile int *)&(v)->counter) = (i))
#define atomic64_set(v,i) ((*(volatile long *)&(v)->counter) = (i))
Thanx, Paul
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 0/24] make atomic_read() behave consistently across all architectures
From: Chris Snook @ 2007-08-09 14:29 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-kernel, linux-arch, torvalds, netdev, akpm, ak,
heiko.carstens, davem, schwidefsky, wensong, horms, wjiang,
cfriesen, zlynx, rpjday, jesper.juhl
In-Reply-To: <200708091441.28511.arnd@arndb.de>
Arnd Bergmann wrote:
> 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 <><
a) chicken and egg: asm-generic/atomic.h depends on definitions in asm/atomic.h
If you can find a way to reshuffle the code and make it simpler, I personally am
all for it. I'm skeptical that you'll get much to show for the effort.
b) The definitions aren't precisely identical between all architectures, so it
would be a mess of special cases, which gets us right back to where we are now.
-- Chris
^ permalink raw reply
* [PATCH 24/24] document volatile atomic_read() behavior
From: Chris Snook @ 2007-08-09 14: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>
Update atomic_ops.txt to reflect the newly consistent behavior of
atomic_read(), and to note that volatile (in declarations) is now
considered harmful.
Signed-off-by: Chris Snook <csnook@redhat.com>
--- linux-2.6.23-rc2-orig/Documentation/atomic_ops.txt 2007-07-08 19:32:17.000000000 -0400
+++ linux-2.6.23-rc2/Documentation/atomic_ops.txt 2007-08-09 08:24:32.000000000 -0400
@@ -12,7 +12,7 @@
C integer type will fail. Something like the following should
suffice:
- typedef struct { volatile int counter; } atomic_t;
+ typedef struct { int counter; } atomic_t;
The first operations to implement for atomic_t's are the
initializers and plain reads.
@@ -38,9 +38,17 @@
Next, we have:
- #define atomic_read(v) ((v)->counter)
+ #define atomic_read(v) (*(volatile int *)&(v)->counter)
-which simply reads the current value of the counter.
+which reads the counter as though it were volatile. This prevents the
+compiler from optimizing away repeated atomic_read() invocations without
+requiring a more expensive barrier(). Historically this has been
+accomplished by declaring the counter itself to be volatile, but the
+ambiguity of the C standard on the semantics of volatile make this practice
+vulnerable to overly creative interpretation by compilers. Explicit
+casting in atomic_read() ensures consistent behavior across architectures
+and compilers. Even with this convenience in atomic_read(), busy-waiters
+should call cpu_relax().
Now, we move onto the actual atomic operation interfaces.
^ permalink raw reply
* [PATCH 1/1] af_packet: don't enable timestamps in mmap'ed sockets
From: Unai Uribarri @ 2007-08-09 14:21 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel
Hello folks,
I've discovered two strange behaviours (bugs?) about timestamp
generation:
1. If a program opens an AF_PACKET socket and setup a reception ring
with setsockopt(sock, SOL_PACKET, PACKET_RX_RING), timestamps are
automatically (re)enabled at the reception of every packet.
2. Setting SOL_SOCKET/SO_TIMESTAMP to 0 doesn't disables timestamp
generation. Every skb continues begin timestamped until you close the
socket that activated it.
Timestamp generation is a heavy task that is consuming more than 50% of
the CPU (using ACPI PM clock) and is currently the bottleneck in my
packet capturing application.
The attached patch removes the automatic timestamp activation, that
only mmap'ed AF_PACKET sockets perform. I known it can break user
applications, but I believe that it's the correct solution.
I will be very pleased to receive any feedback.
Signed-off-by: Unai Uribarri <unai.uribarri@optenet.com>
---
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 1322d62..a4f2da3 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -640,10 +640,6 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev, struct packe
h->tp_snaplen = snaplen;
h->tp_mac = macoff;
h->tp_net = netoff;
- if (skb->tstamp.tv64 == 0) {
- __net_timestamp(skb);
- sock_enable_timestamp(sk);
- }
tv = ktime_to_timeval(skb->tstamp);
h->tp_sec = tv.tv_sec;
h->tp_usec = tv.tv_usec;
^ permalink raw reply related
* [PATCH 23/24] make atomic_read() behave consistently on xtensa
From: Chris Snook @ 2007-08-09 14:20 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 xtensa.
Signed-off-by: Chris Snook <csnook@redhat.com>
--- linux-2.6.23-rc2-orig/include/asm-xtensa/atomic.h 2007-07-08 19:32:17.000000000 -0400
+++ linux-2.6.23-rc2/include/asm-xtensa/atomic.h 2007-08-09 07:54:59.000000000 -0400
@@ -15,7 +15,7 @@
#include <linux/stringify.h>
-typedef struct { volatile int counter; } atomic_t;
+typedef struct { int counter; } atomic_t;
#ifdef __KERNEL__
#include <asm/processor.h>
@@ -47,7 +47,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
* [PATCH 22/24] make atomic_read() behave consistently on x86_64
From: Chris Snook @ 2007-08-09 14:18 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() consistent with atomic64_read() and other architectures
on x86_64.
Signed-off-by: Chris Snook <csnook@redhat.com>
--- linux-2.6.23-rc2-orig/include/asm-x86_64/atomic.h 2007-07-08 19:32:17.000000000 -0400
+++ linux-2.6.23-rc2/include/asm-x86_64/atomic.h 2007-08-09 07:51:40.000000000 -0400
@@ -32,7 +32,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
@@ -206,7 +206,7 @@ static __inline__ int atomic_sub_return(
/* An 64bit atomic type */
-typedef struct { volatile long counter; } atomic64_t;
+typedef struct { long counter; } atomic64_t;
#define ATOMIC64_INIT(i) { (i) }
@@ -217,7 +217,7 @@ typedef struct { volatile long counter;
* Atomically reads the value of @v.
* Doesn't imply a read memory barrier.
*/
-#define atomic64_read(v) ((v)->counter)
+#define atomic64_read(v) (*(volatile long *)&(v)->counter)
/**
* atomic64_set - set atomic64 variable
^ permalink raw reply
* [PATCH 21/24] make atomic_read() behave consistently on v850
From: Chris Snook @ 2007-08-09 14:16 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 v850. 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-v850/atomic.h 2007-07-08 19:32:17.000000000 -0400
+++ linux-2.6.23-rc2/include/asm-v850/atomic.h 2007-08-09 07:50:15.000000000 -0400
@@ -27,7 +27,11 @@ typedef struct { int counter; } atomic_t
#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)
#define atomic_set(v,i) (((v)->counter) = (i))
static inline int atomic_add_return (int i, volatile atomic_t *v)
^ permalink raw reply
* [PATCH 20/24] make atomic_read() behave consistently on sparc
From: Chris Snook @ 2007-08-09 14: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
From: Chris Snook <csnook@redhat.com>
Purify volatile use for atomic_t on sparc.
Leave atomic24_t alone, since it's only used by sparc-specific code.
Signed-off-by: Chris Snook <csnook@redhat.com>
--- linux-2.6.23-rc2-orig/include/asm-sparc/atomic.h 2007-07-08 19:32:17.000000000 -0400
+++ linux-2.6.23-rc2/include/asm-sparc/atomic.h 2007-08-09 07:29:31.000000000 -0400
@@ -13,7 +13,7 @@
#include <linux/types.h>
-typedef struct { volatile int counter; } atomic_t;
+typedef struct { int counter; } atomic_t;
#ifdef __KERNEL__
@@ -61,7 +61,11 @@ extern int atomic_cmpxchg(atomic_t *, in
extern int atomic_add_unless(atomic_t *, int, int);
extern void atomic_set(atomic_t *, int);
-#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_add(i, v) ((void)__atomic_add_return( (int)(i), (v)))
#define atomic_sub(i, v) ((void)__atomic_add_return(-(int)(i), (v)))
^ permalink raw reply
* [PATCH 19/24] make atomic_read() behave consistently on sparc64
From: Chris Snook @ 2007-08-09 14:12 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 sparc64.
Signed-off-by: Chris Snook <csnook@redhat.com>
--- linux-2.6.23-rc2-orig/include/asm-sparc64/atomic.h 2007-07-08 19:32:17.000000000 -0400
+++ linux-2.6.23-rc2/include/asm-sparc64/atomic.h 2007-08-09 07:48:01.000000000 -0400
@@ -11,14 +11,18 @@
#include <linux/types.h>
#include <asm/system.h>
-typedef struct { volatile int counter; } atomic_t;
-typedef struct { volatile __s64 counter; } atomic64_t;
+typedef struct { int counter; } atomic_t;
+typedef struct { __s64 counter; } atomic64_t;
#define ATOMIC_INIT(i) { (i) }
#define ATOMIC64_INIT(i) { (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 int *)&(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
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