Re: [PATCH tip/locking/core v9 2/6] locking/qspinlock: prefetch next node cacheline

[Waiman Long]

On 11/02/2015 05:54 PM, Peter Zijlstra wrote:
> On Mon, Nov 02, 2015 at 05:36:26PM +0100, Peter Zijlstra wrote:
> > On Fri, Oct 30, 2015 at 07:26:33PM -0400, Waiman Long wrote:
> > > @@ -426,6 +437,15 @@ queue:
> > >   		cpu_relax();
> > >
> > >   	/*
> > > +	 * If the next pointer is defined, we are not tail anymore.
> > > +	 * In this case, claim the spinlock&  release the MCS lock.
> > > +	 */
> > > +	if (next) {
> > > +		set_locked(lock);
> > > +		goto mcs_unlock;
> > > +	}
> > > +
> > > +	/*
> > >   	 * claim the lock:
> > >   	 *
> > >   	 * n,0,0 ->  0,0,1 : lock, uncontended
> > > @@ -458,6 +478,7 @@ queue:
> > >   	while (!(next = READ_ONCE(node->next)))
> > >   		cpu_relax();
> > >
> > > +mcs_unlock:
> > >   	arch_mcs_spin_unlock_contended(&next->locked);
> > >   	pv_kick_node(lock, next);
> > This however appears an independent optimization. Is it worth it? Would
> > we not already have observed a val != tail in this case? At which point
> > we're just adding extra code for no gain.
> >
> > That is, if we observe @next, must we then not also observe val != tail?
> Not quite; the ordering is the other way around. If we observe next we
> must also observe val != tail. But its a narrow thing. Is it really
> worth it?

If we observe next, we will observe val != tail sooner or later. It is 
not possible for it to clear the tail code in the lock. The tail xchg 
will guarantee that.

Another alternative is to do something like

+    if (!next)
         while (!(next = READ_ONCE(node->next)))
            cpu_relax();

Please let me know if that is more acceptable to you.

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/