Re: [PATCH v2] locking/pvqspinlock: Add lock holder CPU argument to pv_wait()

From: Waiman Long
Date: Wed Apr 20 2016 - 13:51:01 EST


On 04/20/2016 10:19 AM, Peter Zijlstra wrote:
On Wed, Apr 20, 2016 at 10:15:09PM +0800, Pan Xinhui wrote:
So there is such case that we search the whole hashtable and the lock is not found. :(
Waiman assume that if l = null, the lock is not stored. however the lock might be there actually.
But to avoid the worst case I just mentioned above, it can quickly finish the lookup.

+
+ /*
+ * We try to locate the queue head pv_node by looking
+ * up the hash table. If it is not found, use the
+ * CPU in the previous node instead.
+ */
+ hn = pv_lookup_hash(lock);
+ if (!hn)
+ hn = pn;
This is potentially expensive... it does not explain why this lookup can
fail etc.. nor mentioned that lock stealing caveat.

Yes, it's expensive. Normally, PPC phyp don't always need the correct
holder. That means current vcpu can just give up its slice. There is
one lpar hvcall H_CONFER. I paste some spec below.
Ok, so if we can indeed scan the _entire_ hashtable, then we really
should not have that in common code. That's seriously expensive.

No, it should not scan the entire hashtable and there is no point in doing so.

Cheers,
Longman