Re: [Xen-devel] [PATCH 8/9] qspinlock: Generic paravirt support

2015-04-03 Thread Peter Zijlstra
On Thu, Apr 02, 2015 at 09:48:34PM +0200, Peter Zijlstra wrote: @@ -158,20 +257,20 @@ static void pv_wait_head(struct qspinloc void __pv_queue_spin_unlock(struct qspinlock *lock) { struct __qspinlock *l = (void *)lock; + struct pv_hash_bucket *hb; if (xchg(l-locked, 0)

Re: [Xen-devel] [PATCH 8/9] qspinlock: Generic paravirt support

2015-04-02 Thread Waiman Long
On 04/02/2015 03:48 PM, Peter Zijlstra wrote: On Thu, Apr 02, 2015 at 07:20:57PM +0200, Peter Zijlstra wrote: pv_wait_head(): pv_hash() /* MB as per cmpxchg */ cmpxchg(l-locked, _Q_LOCKED_VAL, _Q_SLOW_VAL); VS __pv_queue_spin_unlock(): if (xchg(l-locked, 0)

Re: [Xen-devel] [PATCH 8/9] qspinlock: Generic paravirt support

2015-04-02 Thread Peter Zijlstra
On Thu, Apr 02, 2015 at 12:28:30PM -0400, Waiman Long wrote: On 04/01/2015 05:03 PM, Peter Zijlstra wrote: On Wed, Apr 01, 2015 at 03:58:58PM -0400, Waiman Long wrote: On 04/01/2015 02:48 PM, Peter Zijlstra wrote: I am sorry that I don't quite get what you mean here. My point is that in the

Re: [Xen-devel] [PATCH 8/9] qspinlock: Generic paravirt support

2015-04-02 Thread Waiman Long
On 04/01/2015 05:03 PM, Peter Zijlstra wrote: On Wed, Apr 01, 2015 at 03:58:58PM -0400, Waiman Long wrote: On 04/01/2015 02:48 PM, Peter Zijlstra wrote: I am sorry that I don't quite get what you mean here. My point is that in the hashing step, a cpu will need to scan an empty bucket to put the

Re: [Xen-devel] [PATCH 8/9] qspinlock: Generic paravirt support

2015-04-01 Thread Waiman Long
On 03/19/2015 08:25 AM, Peter Zijlstra wrote: On Thu, Mar 19, 2015 at 11:12:42AM +0100, Peter Zijlstra wrote: So I was now thinking of hashing the lock pointer; let me go and quickly put something together. A little something like so; ideally we'd allocate the hashtable since NR_CPUS is kinda

Re: [Xen-devel] [PATCH 8/9] qspinlock: Generic paravirt support

2015-04-01 Thread Peter Zijlstra
On Wed, Apr 01, 2015 at 12:20:30PM -0400, Waiman Long wrote: After more careful reading, I think the assumption that the presence of an unused bucket means there is no match is not true. Consider the scenario: 1. cpu 0 puts lock1 into hb[0] 2. cpu 1 puts lock2 into hb[1] 3. cpu 2 clears

Re: [Xen-devel] [PATCH 8/9] qspinlock: Generic paravirt support

2015-04-01 Thread Peter Zijlstra
On Wed, Apr 01, 2015 at 07:12:23PM +0200, Peter Zijlstra wrote: On Wed, Apr 01, 2015 at 12:20:30PM -0400, Waiman Long wrote: After more careful reading, I think the assumption that the presence of an unused bucket means there is no match is not true. Consider the scenario: 1. cpu 0 puts

Re: [Xen-devel] [PATCH 8/9] qspinlock: Generic paravirt support

2015-04-01 Thread Peter Zijlstra
On Wed, Apr 01, 2015 at 07:42:39PM +0200, Peter Zijlstra wrote: Hohumm.. time to think more I think ;-) So bear with me, I've not really pondered this well so it could be full of holes (again). After the cmpxchg(l-locked, _Q_LOCKED_VAL, _Q_SLOW_VAL) succeeds the spin_unlock() must do the

Re: [Xen-devel] [PATCH 8/9] qspinlock: Generic paravirt support

2015-04-01 Thread Waiman Long
On 04/01/2015 02:17 PM, Peter Zijlstra wrote: On Wed, Apr 01, 2015 at 07:42:39PM +0200, Peter Zijlstra wrote: Hohumm.. time to think more I think ;-) So bear with me, I've not really pondered this well so it could be full of holes (again). After the cmpxchg(l-locked, _Q_LOCKED_VAL,

Re: [Xen-devel] [PATCH 8/9] qspinlock: Generic paravirt support

2015-04-01 Thread Peter Zijlstra
On Wed, Apr 01, 2015 at 02:54:45PM -0400, Waiman Long wrote: On 04/01/2015 02:17 PM, Peter Zijlstra wrote: On Wed, Apr 01, 2015 at 07:42:39PM +0200, Peter Zijlstra wrote: Hohumm.. time to think more I think ;-) So bear with me, I've not really pondered this well so it could be full of holes

Re: [Xen-devel] [PATCH 8/9] qspinlock: Generic paravirt support

2015-04-01 Thread Waiman Long
On 04/01/2015 02:48 PM, Peter Zijlstra wrote: On Wed, Apr 01, 2015 at 02:54:45PM -0400, Waiman Long wrote: On 04/01/2015 02:17 PM, Peter Zijlstra wrote: On Wed, Apr 01, 2015 at 07:42:39PM +0200, Peter Zijlstra wrote: Hohumm.. time to think more I think ;-) So bear with me, I've not really

Re: [Xen-devel] [PATCH 8/9] qspinlock: Generic paravirt support

2015-04-01 Thread Waiman Long
On 04/01/2015 01:12 PM, Peter Zijlstra wrote: On Wed, Apr 01, 2015 at 12:20:30PM -0400, Waiman Long wrote: After more careful reading, I think the assumption that the presence of an unused bucket means there is no match is not true. Consider the scenario: 1. cpu 0 puts lock1 into hb[0] 2. cpu

Re: [Xen-devel] [PATCH 8/9] qspinlock: Generic paravirt support

2015-04-01 Thread Peter Zijlstra
On Wed, Apr 01, 2015 at 03:58:58PM -0400, Waiman Long wrote: On 04/01/2015 02:48 PM, Peter Zijlstra wrote: I am sorry that I don't quite get what you mean here. My point is that in the hashing step, a cpu will need to scan an empty bucket to put the lock in. In the interim, an previously used

Re: [Xen-devel] [PATCH 8/9] qspinlock: Generic paravirt support

2015-03-19 Thread Waiman Long
On 03/19/2015 08:25 AM, Peter Zijlstra wrote: On Thu, Mar 19, 2015 at 11:12:42AM +0100, Peter Zijlstra wrote: So I was now thinking of hashing the lock pointer; let me go and quickly put something together. A little something like so; ideally we'd allocate the hashtable since NR_CPUS is kinda

Re: [Xen-devel] [PATCH 8/9] qspinlock: Generic paravirt support

2015-03-19 Thread Peter Zijlstra
On Wed, Mar 18, 2015 at 04:50:37PM -0400, Waiman Long wrote: +this_cpu_write(__pv_lock_wait, lock); We may run into the same problem of needing to have 4 queue nodes per CPU. If an interrupt happens just after the write and before the actual wait and it goes through the same

Re: [Xen-devel] [PATCH 8/9] qspinlock: Generic paravirt support

2015-03-19 Thread Peter Zijlstra
On Thu, Mar 19, 2015 at 01:25:36PM +0100, Peter Zijlstra wrote: +static struct qspinlock **pv_hash(struct qspinlock *lock) +{ + u32 hash = hash_ptr(lock, PV_LOCK_HASH_BITS); + struct pv_hash_bucket *hb, *end; + + if (!hash) + hash = 1; + + hb =

Re: [Xen-devel] [PATCH 8/9] qspinlock: Generic paravirt support

2015-03-18 Thread Waiman Long
On 03/16/2015 09:16 AM, Peter Zijlstra wrote: Implement simple paravirt support for the qspinlock. Provide a separate (second) version of the spin_lock_slowpath for paravirt along with a special unlock path. The second slowpath is generated by adding a few pv hooks to the normal slowpath, but

[Xen-devel] [PATCH 8/9] qspinlock: Generic paravirt support

2015-03-16 Thread Peter Zijlstra
Implement simple paravirt support for the qspinlock. Provide a separate (second) version of the spin_lock_slowpath for paravirt along with a special unlock path. The second slowpath is generated by adding a few pv hooks to the normal slowpath, but where those will compile away for the native