Fix a race in pid generation that causes pids to be reused immediately.
in [Kernel]
|
Prev: [PATCH] zorro: Fix reading of proc/bus/zorro/* in small chunks
Next: Ext2/3 Filesystem Analysis
From: Eric Dumazet on 15 Jun 2010 00:40 Le lundi 14 juin 2010 à 21:21 -0700, Andrew Morton a écrit : > On Tue, 15 Jun 2010 13:26:08 +1000 Paul Mackerras <paulus(a)samba.org> wrote: > > > On Mon, Jun 14, 2010 at 06:55:56PM -0700, Andrew Morton wrote: > > > > > > kernel/sched_clock.c: if (cmpxchg64(&scd->clock, old_clock, clock) != old_cloc > > > > > > I guess that'll flush out any stragglers. > > > > And break most non-x86 32-bit architectures, including 32-bit powerpc. > > If CONFIG_SMP=y, yes. On UP there's a generic implementation > (include/asm-generic/cmpxchg-local.h, include/asm-generic/cmpxchg.h) > > > Fortunately that code is only used if CONFIG_HAVE_UNSTABLE_SCHED_CLOCK > > is set, and it looks like only x86 and ia64 set it. > > > > If that happens then the best fix is for those architectures to get > themselves a cmpxchg64(). Unless for some reason it's simply > unimplementable? Worst case I guess one could use a global spinlock. > Second-worst-case: hashed spinlocks. Hmm, this reminds me a patch I had somewhere, not yet sent to David :) 1) cmpxchg() was not available for all arches, maybe atomic_long_cmpxchg() is ? 2) I am not sure atomic_long_t quarantee that all 32 or 64 bits of the underlying long container are available. Thanks ! diff --git a/net/ipv4/route.c b/net/ipv4/route.c index a291edb..335ca89 100644 --- a/net/ipv4/route.c +++ b/net/ipv4/route.c @@ -1268,17 +1268,18 @@ skip_hashing: void rt_bind_peer(struct rtable *rt, int create) { - static DEFINE_SPINLOCK(rt_peer_lock); struct inet_peer *peer; peer = inet_getpeer(rt->rt_dst, create); - spin_lock_bh(&rt_peer_lock); - if (rt->peer == NULL) { - rt->peer = peer; +#if defined(__HAVE_ARCH_CMPXCHG) + if (!cmpxchg(&rt->peer, NULL, peer)) peer = NULL; - } - spin_unlock_bh(&rt_peer_lock); +#else + BUILD_BUG_ON(sizeof(rt->peer) != sizeof(atomic_long_t)); + if (!atomic_long_cmpxchg((atomic_long_t *)&rt->peer, 0, (long)peer)) + peer = NULL; +#endif if (peer) inet_putpeer(peer); } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Benjamin Herrenschmidt on 15 Jun 2010 03:00 On Mon, 2010-06-14 at 21:21 -0700, Andrew Morton wrote: > If that happens then the best fix is for those architectures to get > themselves a cmpxchg64(). Unless for some reason it's simply > unimplementable? Worst case I guess one could use a global spinlock. > Second-worst-case: hashed spinlocks. Right, ppc32 at least can't so that would be spinlocks... Cheers, Ben. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Paul Mackerras on 15 Jun 2010 03:30 On Mon, Jun 14, 2010 at 09:21:50PM -0700, Andrew Morton wrote: > If that happens then the best fix is for those architectures to get > themselves a cmpxchg64(). Unless for some reason it's simply > unimplementable? Worst case I guess one could use a global spinlock. > Second-worst-case: hashed spinlocks. Spinlocks won't help as long as you can use cmpxchg64 on any old u64 that is also accessed directly. UP can disable interrupts, of course, but for SMP we would have to restrict cmpxchg64 to some type (atomic64_t, maybe) for which you have to use an accessor function to read it, so we can take the spinlock around the reads. I suspect it isn't worth the trouble. Note that I'm talking specifically about cmpxchg64 here, not cmpxchg. Paul. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: tytso on 15 Jun 2010 09:00 On Mon, Jun 14, 2010 at 06:55:56PM -0700, Andrew Morton wrote: > > I think you're probably right, as long as one sticks with 4-byte > scalars. The cmpxchg-is-now-generic change snuck in under the radar > (mine, at least). Hmmm, what about unsigned longs? (Which might be 8 bytes on some architectures....) - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Peter Zijlstra on 15 Jun 2010 10:40
On Mon, 2010-06-14 at 18:55 -0700, Andrew Morton wrote: > > kernel/sched_clock.c: if (cmpxchg64(&scd->clock, old_clock, clock) != old_cloc > > I guess that'll flush out any stragglers. cmpxchg64() is special, at the time i386 didn't handle the 8 byte cmpxchg(), although we could easily make it do today. > I suspect sched_clock.c might be generating fair amounts of code which > UP builds don't need. Only sched_clock_remote() and its caller, something like the below, not much code.. UP machines can still have utterly sucky TSC, although the inter-cpu-drift thing isn't much of an issue ;-) --- kernel/sched_clock.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/kernel/sched_clock.c b/kernel/sched_clock.c index 52f1a14..7ff5b56 100644 --- a/kernel/sched_clock.c +++ b/kernel/sched_clock.c @@ -170,6 +170,7 @@ again: return clock; } +#ifdef CONFIG_SMP static u64 sched_clock_remote(struct sched_clock_data *scd) { struct sched_clock_data *my_scd = this_scd(); @@ -205,6 +206,7 @@ again: return val; } +#endif /* * Similar to cpu_clock(), but requires local IRQs to be disabled. @@ -226,9 +228,11 @@ u64 sched_clock_cpu(int cpu) scd = cpu_sdc(cpu); +#ifdef CONFIG_SMP if (cpu != smp_processor_id()) clock = sched_clock_remote(scd); else +#endif clock = sched_clock_local(scd); return clock; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |