Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1196096 > unrolled thread

Re: [PATCH tip/core/rcu 19/19] rcu: Add fastpath bypassing funnel locking

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH tip/core/rcu 19/19] rcu: Add fastpath bypassing funnel  locking Peter Zijlstra <peterz@infradead.org> - 2015-07-30 16:50 +0200
    Re: [PATCH tip/core/rcu 19/19] rcu: Add fastpath bypassing funnel  locking "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-07-30 17:40 +0200
      Re: [PATCH tip/core/rcu 19/19] rcu: Add fastpath bypassing funnel  locking Peter Zijlstra <peterz@infradead.org> - 2015-07-30 17:50 +0200
        Re: [PATCH tip/core/rcu 19/19] rcu: Add fastpath bypassing funnel  locking Steven Rostedt <rostedt@goodmis.org> - 2015-08-03 22:10 +0200
          Re: [PATCH tip/core/rcu 19/19] rcu: Add fastpath bypassing funnel  locking Peter Zijlstra <peterz@infradead.org> - 2015-08-03 22:10 +0200
      Re: [PATCH tip/core/rcu 19/19] rcu: Add fastpath bypassing funnel  locking "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2015-07-31 18:00 +0200

#1196096 — Re: [PATCH tip/core/rcu 19/19] rcu: Add fastpath bypassing funnel locking

On Fri, Jul 17, 2015 at 04:29:24PM -0700, Paul E. McKenney wrote:

>  	/*
> +	 * First try directly acquiring the root lock in order to reduce
> +	 * latency in the common case where expedited grace periods are
> +	 * rare.  We check mutex_is_locked() to avoid pathological levels of
> +	 * memory contention on ->exp_funnel_mutex in the heavy-load case.
> +	 */
> +	rnp0 = rcu_get_root(rsp);
> +	if (!mutex_is_locked(&rnp0->exp_funnel_mutex)) {
> +		if (mutex_trylock(&rnp0->exp_funnel_mutex)) {
> +			if (sync_exp_work_done(rsp, rnp0, NULL,
> +					       &rsp->expedited_workdone0, s))
> +				return NULL;
> +			return rnp0;
> +		}
> +	}

So our 'new' locking primitives do things like:

static __always_inline int queued_spin_trylock(struct qspinlock *lock)
{
        if (!atomic_read(&lock->val) &&
           (atomic_cmpxchg(&lock->val, 0, _Q_LOCKED_VAL) == 0))
                return 1;
        return 0;
}

mutexes do not do this.

Now I suppose the question is, does that extra read slow down the
(common) uncontended case? (remember, we should optimize locks for the
uncontended case, heavy lock contention should be fixed with better
locking schemes, not lock implementations).

Davidlohr, Waiman, do we have data on this?

If the extra read before the cmpxchg() does not hurt, we should do the
same for mutex and make the above redundant.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1196144

On Thu, Jul 30, 2015 at 04:44:55PM +0200, Peter Zijlstra wrote:
> On Fri, Jul 17, 2015 at 04:29:24PM -0700, Paul E. McKenney wrote:
> 
> >  	/*
> > +	 * First try directly acquiring the root lock in order to reduce
> > +	 * latency in the common case where expedited grace periods are
> > +	 * rare.  We check mutex_is_locked() to avoid pathological levels of
> > +	 * memory contention on ->exp_funnel_mutex in the heavy-load case.
> > +	 */
> > +	rnp0 = rcu_get_root(rsp);
> > +	if (!mutex_is_locked(&rnp0->exp_funnel_mutex)) {
> > +		if (mutex_trylock(&rnp0->exp_funnel_mutex)) {
> > +			if (sync_exp_work_done(rsp, rnp0, NULL,
> > +					       &rsp->expedited_workdone0, s))
> > +				return NULL;
> > +			return rnp0;
> > +		}
> > +	}
> 
> So our 'new' locking primitives do things like:
> 
> static __always_inline int queued_spin_trylock(struct qspinlock *lock)
> {
>         if (!atomic_read(&lock->val) &&
>            (atomic_cmpxchg(&lock->val, 0, _Q_LOCKED_VAL) == 0))
>                 return 1;
>         return 0;
> }
> 
> mutexes do not do this.
> 
> Now I suppose the question is, does that extra read slow down the
> (common) uncontended case? (remember, we should optimize locks for the
> uncontended case, heavy lock contention should be fixed with better
> locking schemes, not lock implementations).
> 
> Davidlohr, Waiman, do we have data on this?
> 
> If the extra read before the cmpxchg() does not hurt, we should do the
> same for mutex and make the above redundant.

I am pretty sure that different hardware wants it done differently.  :-/
So I agree that hard data would be good.

I could probably further optimize the RCU code by checking for a
single-node tree, but I am not convinced that this is worthwhile.
However, skipping three cache misses in the uncontended case is
definitely worthwhile, hence this patch.  ;-)

							Thanx, Paul

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1196149

On Thu, Jul 30, 2015 at 08:34:52AM -0700, Paul E. McKenney wrote:
> > If the extra read before the cmpxchg() does not hurt, we should do the
> > same for mutex and make the above redundant.
> 
> I am pretty sure that different hardware wants it done differently.  :-/
> So I agree that hard data would be good.
> 
> I could probably further optimize the RCU code by checking for a
> single-node tree, but I am not convinced that this is worthwhile.
> However, skipping three cache misses in the uncontended case is
> definitely worthwhile, hence this patch.  ;-)

I was mostly talking about the !mutex_is_locked() && mutex_try_lock()
thing. The fast path thing makes sense.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1199235

On Thu, 30 Jul 2015 17:40:01 +0200
Peter Zijlstra <peterz@infradead.org> wrote:

> On Thu, Jul 30, 2015 at 08:34:52AM -0700, Paul E. McKenney wrote:
> > > If the extra read before the cmpxchg() does not hurt, we should do the
> > > same for mutex and make the above redundant.
> > 
> > I am pretty sure that different hardware wants it done differently.  :-/
> > So I agree that hard data would be good.
> > 
> > I could probably further optimize the RCU code by checking for a
> > single-node tree, but I am not convinced that this is worthwhile.
> > However, skipping three cache misses in the uncontended case is
> > definitely worthwhile, hence this patch.  ;-)
> 
> I was mostly talking about the !mutex_is_locked() && mutex_try_lock()
> thing. The fast path thing makes sense.

Note, mutex does do this for the optimistic spin. See
mutex_try_to_aquire().

-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1199236

On Mon, Aug 03, 2015 at 04:05:33PM -0400, Steven Rostedt wrote:
> On Thu, 30 Jul 2015 17:40:01 +0200
> Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > On Thu, Jul 30, 2015 at 08:34:52AM -0700, Paul E. McKenney wrote:
> > > > If the extra read before the cmpxchg() does not hurt, we should do the
> > > > same for mutex and make the above redundant.
> > > 
> > > I am pretty sure that different hardware wants it done differently.  :-/
> > > So I agree that hard data would be good.
> > > 
> > > I could probably further optimize the RCU code by checking for a
> > > single-node tree, but I am not convinced that this is worthwhile.
> > > However, skipping three cache misses in the uncontended case is
> > > definitely worthwhile, hence this patch.  ;-)
> > 
> > I was mostly talking about the !mutex_is_locked() && mutex_try_lock()
> > thing. The fast path thing makes sense.
> 
> Note, mutex does do this for the optimistic spin. See
> mutex_try_to_aquire().

Right but that's mutex_lock(). mutex_trylock() does not.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1197150

On Thu, Jul 30, 2015 at 06:34:27PM +0200, Peter Zijlstra wrote:
> On Thu, Jul 30, 2015 at 08:34:52AM -0700, Paul E. McKenney wrote:
> > On Thu, Jul 30, 2015 at 04:44:55PM +0200, Peter Zijlstra wrote:
> 
> > > If the extra read before the cmpxchg() does not hurt, we should do the
> > > same for mutex and make the above redundant.
> > 
> > I am pretty sure that different hardware wants it done differently.  :-/
> 
> I think that most archs won't notice since any RmW includes a load of
> that variable anyhow. The only case where it can matter is if the RmW is
> done outside of the normal cache hierarchy -- like on Power, where the
> ll/sc bypasses the L1.

Some years back, AMD and Intel variants of x86 had different preferences
on this matter.  Timings indicated that one or the other of them (I
cannot recall which) would get the cacheline shared, then have to get
it exclusive, while the other would get it exclusive to begin with.

I honestly do not know what the preferences of current Power hardware
might be.

							Thanx, Paul

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web