Messages in this thread | | | Date | Mon, 02 Jun 2014 13:29:54 -0400 | From | Waiman Long <> | Subject | Re: [PATCH v2] introduce atomic_pointer to fix a race condition in cancelable mcs spinlocks |
| |
On 06/02/2014 12:43 PM, Paul E. McKenney wrote: > On Mon, Jun 02, 2014 at 06:25:25PM +0200, Peter Zijlstra wrote: >> On Mon, Jun 02, 2014 at 12:00:45PM -0400, Mikulas Patocka wrote: >>> struct optimistic_spin_queue { >>> - struct optimistic_spin_queue *next, *prev; >>> + atomic_pointer(struct optimistic_spin_queue *) next; >>> + struct optimistic_spin_queue *prev; >>> int locked; /* 1 if lock acquired */ >>> }; >>> >>> Index: linux-3.15-rc8/include/asm-generic/atomic-long.h >>> =================================================================== >>> --- linux-3.15-rc8.orig/include/asm-generic/atomic-long.h 2014-06-02 17:11:17.000000000 +0200 >>> +++ linux-3.15-rc8/include/asm-generic/atomic-long.h 2014-06-02 17:11:50.000000000 +0200 >>> @@ -255,4 +255,31 @@ static inline long atomic_long_add_unles >>> >>> #endif /* BITS_PER_LONG == 64 */ >>> >>> +#define atomic_pointer(type) \ >>> +union { \ >>> + atomic_long_t __a; \ >>> + type __t; \ >>> + char __check_sizeof[sizeof(type) == sizeof(long) ? 1 : -1]; \ >>> +} >> That's still entirely disgusting, and afaict entirely redundant. You can >> do that test in the operators below just fine. >> >>> +#define ATOMIC_POINTER_INIT(i) { .__t = (i) } >>> + >>> +#define atomic_pointer_read(v) ((typeof((v)->__t))atomic_long_read(&(v)->__a)) >>> + >>> +#define atomic_pointer_set(v, i) ({ \ >>> + typeof((v)->__t) __i = (i); \ >>> + atomic_long_set(&(v)->__a, (long)(__i)); \ >>> +}) >>> + >>> +#define atomic_pointer_xchg(v, i) ({ \ >>> + typeof((v)->__t) __i = (i); \ >>> + (typeof((v)->__t))atomic_long_xchg(&(v)->__a, (long)(__i)); \ >>> +}) >>> + >>> +#define atomic_pointer_cmpxchg(v, old, new) ({ \ >>> + typeof((v)->__t) __old = (old); \ >>> + typeof((v)->__t) __new = (new); \ >>> + (typeof((v)->__t))atomic_long_cmpxchg(&(v)->__a, (long)(__old), (long)(__new));\ >>> +}) >> 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. > That is an excellent point for this particular issue. Do parisc systems > really support enough CPUs to make queued spinlocks worthwhile? If not, > maybe we should just have parisc stick with traditional spinlocks.
The operation in question is the optimistic spinning code of mutex which is currently active, I think, for all architectures. It is not related to the queued spinlock, though it will have the same problem.
Yes, by disabling the MUTEX_SPIN_ON_OWNER config variable from PA-RISC, we can disable optimistic spinning.
-Longman
| |