Re: [PATCH v7 4/5] locking/pvqspinlock: Allow 1 lock stealing attempt

[Waiman Long]

On 10/14/2015 05:28 AM, Peter Zijlstra wrote:
> On Tue, Oct 13, 2015 at 04:50:25PM -0400, Waiman Long wrote:
> > On 10/13/2015 03:56 PM, Peter Zijlstra wrote:
> > > So the below is exactly duplicated from the normal slowpath, so why
> > > don't you keep that there?
> > >
> > > It would get you something like:
> > >
> > > 	if (pv_wait_head_or_steal(..))
> > > 		goto stolen;
> > >
> > >
> > > stolen:
> > > > +	/*
> > > > +	 * contended path; wait for next, release.
> > > > +	 */
> > > > +	while (!(next = READ_ONCE(node->next)))
> > > > +		cpu_relax();
> > > > +
> > > > +	arch_mcs_spin_unlock_contended(&next->locked);
> > > > +	pv_kick_node(lock, next);
> > > release:
> > > 	...
> > Yes, it is largely the same. I thought that you don't like too much change
> > in the logic flow of the generic qspinlock code. I will make the change in
> > the next revision.
> Well, you already put the branch in there, the only difference here is
> an 'extra' label. OTOH that extra label avoids duplicating some hairy
> code. So over all I would say its a definite win.
>
> And its easy to see it will compile away on the native case where:
>
>   #define pv_wait_head_or_steal(l, n, t) (false)

I have done it in a different way to avoid duplicating code. I think I 
am not going to use the pv_wait_head_or_steal name after all as I don't 
consider acquiring the lock by the queue head CPU is lock stealing. I 
will skip "and" and name it as pv_wait_head_lock instead. Please let me 
know if you have a better idea.

Cheers,
Longman
--
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/