Re: [PATCH] fix a race condition in cancelable mcs spinlocks

2014-06-04 Thread Mikulas Patocka
On Tue, 3 Jun 2014, Peter Zijlstra wrote: > On Mon, Jun 02, 2014 at 12:56:40PM -0700, James Bottomley wrote: > > Architecturally, there is a way we could emulate the atomic exchange > > instructions. We could have a special section of memory that always > > triggers a page trap. In the Q state

Re: [PATCH] fix a race condition in cancelable mcs spinlocks

2014-06-03 Thread Peter Zijlstra
On Mon, Jun 02, 2014 at 12:56:40PM -0700, James Bottomley wrote: > Architecturally, there is a way we could emulate the atomic exchange > instructions. We could have a special section of memory that always > triggers a page trap. In the Q state dtlb trap handlers we could > recognise the "atomic"

Re: [PATCH] fix a race condition in cancelable mcs spinlocks

2014-06-02 Thread James Bottomley
On Sun, 2014-06-01 at 23:30 +0200, Peter Zijlstra wrote: > On Sun, Jun 01, 2014 at 04:46:26PM -0400, John David Anglin wrote: > > On 1-Jun-14, at 3:20 PM, Peter Zijlstra wrote: > > > > >>If you write to some variable with ACCESS_ONCE and use cmpxchg or xchg > > >>at > > >>the same time, you break

Re: [PATCH] fix a race condition in cancelable mcs spinlocks

2014-06-02 Thread Paul E. McKenney
On Mon, Jun 02, 2014 at 11:57:14AM -0400, Mikulas Patocka wrote: > > > On Mon, 2 Jun 2014, Paul E. McKenney wrote: > > > On Mon, Jun 02, 2014 at 05:19:39AM -0400, Mikulas Patocka wrote: > > > > > > > > > On Sun, 1 Jun 2014, Peter Zijlstra wrote: > > > > > > > On Sun, Jun 01, 2014 at 04:46:26P

Re: [PATCH] fix a race condition in cancelable mcs spinlocks

2014-06-02 Thread Mikulas Patocka
On Mon, 2 Jun 2014, Paul E. McKenney wrote: > On Mon, Jun 02, 2014 at 05:19:39AM -0400, Mikulas Patocka wrote: > > > > > > On Sun, 1 Jun 2014, Peter Zijlstra wrote: > > > > > On Sun, Jun 01, 2014 at 04:46:26PM -0400, John David Anglin wrote: > > > > On 1-Jun-14, at 3:20 PM, Peter Zijlstra wro

Re: [PATCH] fix a race condition in cancelable mcs spinlocks

2014-06-02 Thread John David Anglin
On 6/2/2014 10:02 AM, Mikulas Patocka wrote: On Mon, 2 Jun 2014, Mikulas Patocka wrote: On Sun, 1 Jun 2014, John David Anglin wrote: On 1-Jun-14, at 3:20 PM, Peter Zijlstra wrote: If you write to some variable with ACCESS_ONCE and use cmpxchg or xchg at the same time, you break it. ACCESS

Re: [PATCH] fix a race condition in cancelable mcs spinlocks

2014-06-02 Thread Jason Low
On Mon, 2014-06-02 at 10:14 -0400, Waiman Long wrote: > On 06/01/2014 01:53 PM, Mikulas Patocka wrote: > > struct optimistic_spin_queue { > > - struct optimistic_spin_queue *next, *prev; > > + atomic_pointer_t next; > > + struct optimistic_spin_queue *prev; > > int locked; /* 1 if lock

Re: [PATCH] fix a race condition in cancelable mcs spinlocks

2014-06-02 Thread Waiman Long
On 06/01/2014 01:53 PM, Mikulas Patocka wrote: The code in kernel/locking/mcs_spinlock.c is broken. The osq_lock and osq_unlock functions aren't the only ones that need to be changed, the mcs_spin_lock and mcs_spin_unlock have exactly the same problem. There aren't certainly problems in other

Re: [PATCH] fix a race condition in cancelable mcs spinlocks

2014-06-02 Thread Mikulas Patocka
On Mon, 2 Jun 2014, Mikulas Patocka wrote: > > > On Sun, 1 Jun 2014, John David Anglin wrote: > > > On 1-Jun-14, at 3:20 PM, Peter Zijlstra wrote: > > > > > > If you write to some variable with ACCESS_ONCE and use cmpxchg or xchg > > > > at > > > > the same time, you break it. ACCESS_ONCE d

Re: [PATCH] fix a race condition in cancelable mcs spinlocks

2014-06-02 Thread Mikulas Patocka
On Sun, 1 Jun 2014, John David Anglin wrote: > On 1-Jun-14, at 3:20 PM, Peter Zijlstra wrote: > > > > If you write to some variable with ACCESS_ONCE and use cmpxchg or xchg at > > > the same time, you break it. ACCESS_ONCE doesn't take the hashed spinlock, > > > so, in this case, cmpxchg or xch

Re: [PATCH] fix a race condition in cancelable mcs spinlocks

2014-06-02 Thread Paul E. McKenney
On Mon, Jun 02, 2014 at 05:19:39AM -0400, Mikulas Patocka wrote: > > > On Sun, 1 Jun 2014, Peter Zijlstra wrote: > > > On Sun, Jun 01, 2014 at 04:46:26PM -0400, John David Anglin wrote: > > > On 1-Jun-14, at 3:20 PM, Peter Zijlstra wrote: > > > > > > >>If you write to some variable with ACCESS_

Re: [PATCH] fix a race condition in cancelable mcs spinlocks

2014-06-02 Thread Mikulas Patocka
On Sun, 1 Jun 2014, Peter Zijlstra wrote: > On Sun, Jun 01, 2014 at 01:53:11PM -0400, Mikulas Patocka wrote: > > PA-RISC doesn't have xchg or cmpxchg atomic instructions like other > > processors. It only has ldcw and ldcd instructions that load a word (or > > doubleword) from memory and atomica

Re: [PATCH] fix a race condition in cancelable mcs spinlocks

2014-06-02 Thread Mikulas Patocka
On Sun, 1 Jun 2014, Peter Zijlstra wrote: > On Sun, Jun 01, 2014 at 04:46:26PM -0400, John David Anglin wrote: > > On 1-Jun-14, at 3:20 PM, Peter Zijlstra wrote: > > > > >>If you write to some variable with ACCESS_ONCE and use cmpxchg or xchg > > >>at > > >>the same time, you break it. ACCESS_O

Re: [PATCH] fix a race condition in cancelable mcs spinlocks

2014-06-01 Thread Paul E. McKenney
On Sun, Jun 01, 2014 at 11:30:03PM +0200, Peter Zijlstra wrote: > On Sun, Jun 01, 2014 at 04:46:26PM -0400, John David Anglin wrote: > > On 1-Jun-14, at 3:20 PM, Peter Zijlstra wrote: > > > > >>If you write to some variable with ACCESS_ONCE and use cmpxchg or xchg > > >>at > > >>the same time, you

Re: [PATCH] fix a race condition in cancelable mcs spinlocks

2014-06-01 Thread Peter Zijlstra
On Sun, Jun 01, 2014 at 04:46:26PM -0400, John David Anglin wrote: > On 1-Jun-14, at 3:20 PM, Peter Zijlstra wrote: > > >>If you write to some variable with ACCESS_ONCE and use cmpxchg or xchg > >>at > >>the same time, you break it. ACCESS_ONCE doesn't take the hashed > >>spinlock, > >>so, in this

Re: [PATCH] fix a race condition in cancelable mcs spinlocks

2014-06-01 Thread John David Anglin
On 1-Jun-14, at 3:20 PM, Peter Zijlstra wrote: If you write to some variable with ACCESS_ONCE and use cmpxchg or xchg at the same time, you break it. ACCESS_ONCE doesn't take the hashed spinlock, so, in this case, cmpxchg or xchg isn't really atomic at all. And this is really the first pla

Re: [PATCH] fix a race condition in cancelable mcs spinlocks

2014-06-01 Thread Peter Zijlstra
On Sun, Jun 01, 2014 at 01:53:11PM -0400, Mikulas Patocka wrote: > PA-RISC doesn't have xchg or cmpxchg atomic instructions like other > processors. It only has ldcw and ldcd instructions that load a word (or > doubleword) from memory and atomically store zero at the same location. > These instruct

[PATCH] fix a race condition in cancelable mcs spinlocks

2014-06-01 Thread Mikulas Patocka
The cancelable MCS spinlocks introduced in fb0527bd5ea99bfeb2dd91e3c1433ecf745d6b99 break the kernel on PA-RISC. How to reproduce: * Use a machine with two dual-core PA-8900 processors. * Run the LVM testsuite and compile the kernel in an endless loop at the same time. * Wait for an hour or two