Re: [PATCH v2] introduce atomic_pointer to fix a race condition in cancelable mcs spinlocks
From: Mikulas Patocka
Date: Mon Jun 02 2014 - 16:47:50 EST
On Mon, 2 Jun 2014, Linus Torvalds wrote:
> On Mon, Jun 2, 2014 at 9:25 AM, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> > And I can't say I'm a particular fan of these ops either, as alternative
> > I'm almost inclined to just exclude parisc from using opt spinning.
> Please do.
> There is no way in hell that we should introduce a magic new
> atomic_pointer thing for parisc. And the idea somebody had to change
> ACCESS_ONCE() for PA-RISC (I'm not going to go back to find who to
> blame) is just horribly wrong too, since it's not even necessary for
> any normal use: the special "load-and-store-zero" instruction isn't
> actually used for "real" data, it's used only for the special
> spinlocks afaik, so doing it for all ACCESS_ONCE() users would be
> wrong even on PA-RISC. For any normal data, the usual "just load the
> value, making sure the compiler doesn't reload it" is perfectly fine -
> even on PA-RISC.
> Now, if PA-RISC was a major architecture, we'd have to figure this
> out. But as it is, PA-RISC is just about the shittiest RISC ever
> invented (with original sparc being a strong contender), and let's
> face it, nobody really uses it. It's a "fun project", but it is not
> something that we should use to mess up either ACCESS_ONCE() or the
> MCS locks.
> Just make PA-RISC use its own locks, not any of the new fancy ones.
And what else do you want to do?
Peter Zijlstra said "I've been using xchg() and cmpxchg() without such
consideration for quite a while." - so it basically implies that the
kernel is full of such races, mcs_spinlock is just the most visible one
that crashes the kernel first.
It's not only parisc - tile32, arc, metag (maybe hexagon) are broken too,
because they don't have cmpxchg in hardware.
We have atomic_t, atomic64_t, atomic_long_t that can be sanely used even
on architectures without hardware cmpxchg - so I ask - why can't we have
atomic_pointer_t with the same semantics? (pointer type conversion issues
can be solved, as it is done in the PATCH v2)
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/