Re: [PATCH 3/8] srcu: Use local_lock() for per-CPU struct srcu_data access

2020-05-24 Thread Paul E. McKenney
On Sun, May 24, 2020 at 09:03:56PM +0200, Sebastian Andrzej Siewior wrote: > On 2020-05-23 17:08:32 [+0200], To Paul E. McKenney wrote: > > On 2020-05-22 10:39:53 [-0700], Paul E. McKenney wrote: > > > It looks good to me, but I have not yet tested it. (Happy to let you > > > take the first crack

Re: [PATCH 3/8] srcu: Use local_lock() for per-CPU struct srcu_data access

2020-05-24 Thread Sebastian Andrzej Siewior
On 2020-05-23 17:08:32 [+0200], To Paul E. McKenney wrote: > On 2020-05-22 10:39:53 [-0700], Paul E. McKenney wrote: > > It looks good to me, but I have not yet tested it. (Happy to let you > > take the first crack at rcutorture in any case, scenarios SRCU-P and > > SRCU-N.) > > on it. |tools/te

Re: [PATCH 3/8] srcu: Use local_lock() for per-CPU struct srcu_data access

2020-05-23 Thread Paul E. McKenney
On Sat, May 23, 2020 at 05:08:31PM +0200, Sebastian Andrzej Siewior wrote: > On 2020-05-22 10:39:53 [-0700], Paul E. McKenney wrote: > > It looks good to me, but I have not yet tested it. (Happy to let you > > take the first crack at rcutorture in any case, scenarios SRCU-P and > > SRCU-N.) > > o

Re: [PATCH 3/8] srcu: Use local_lock() for per-CPU struct srcu_data access

2020-05-23 Thread Sebastian Andrzej Siewior
On 2020-05-22 10:39:53 [-0700], Paul E. McKenney wrote: > It looks good to me, but I have not yet tested it. (Happy to let you > take the first crack at rcutorture in any case, scenarios SRCU-P and > SRCU-N.) on it. > > That check_init_srcu_struct() is needed, because otherwise: > > > > | BUG:

Re: [PATCH 3/8] srcu: Use local_lock() for per-CPU struct srcu_data access

2020-05-22 Thread Paul E. McKenney
On Fri, May 22, 2020 at 05:12:55PM +0200, Sebastian Andrzej Siewior wrote: > On 2020-05-20 11:43:45 [-0700], Paul E. McKenney wrote: > > > > Yes, that CPU's rcu_segcblist structure does need mutual exclusion in > > this case. This is because rcu_segcblist_pend_cbs() looks not just > > at the ->ta

Re: [PATCH 3/8] srcu: Use local_lock() for per-CPU struct srcu_data access

2020-05-22 Thread Sebastian Andrzej Siewior
On 2020-05-20 11:43:45 [-0700], Paul E. McKenney wrote: > > Yes, that CPU's rcu_segcblist structure does need mutual exclusion in > this case. This is because rcu_segcblist_pend_cbs() looks not just > at the ->tails[] pointer, but also at the pointer referenced by the > ->tails[] pointer. This l

Re: [PATCH 3/8] srcu: Use local_lock() for per-CPU struct srcu_data access

2020-05-20 Thread Joel Fernandes
On Wed, May 20, 2020 at 08:28:00PM +0200, Sebastian Andrzej Siewior wrote: > On 2020-05-20 13:42:59 [-0400], Joel Fernandes wrote: > > Hi Sebastian, > Hi Joel, > > > For pointer stability, can we just use get_local_ptr() and put_local_ptr() > > instead of adding an extra lock? This keeps the point

Re: [PATCH 3/8] srcu: Use local_lock() for per-CPU struct srcu_data access

2020-05-20 Thread Uladzislau Rezki
> > I actually found it in RT 4.4 kernel, I thought this was also on newer RT > kernels as well (is that not true anymore?). But yes it was exactly what > Peter said. > I see it also in 5.6.4 linut-rt-devel: #ifdef CONFIG_PREEMPT_RT ... # define get_local_ptr(var) ({ \ migrate_disable(); \ thi

Re: [PATCH 3/8] srcu: Use local_lock() for per-CPU struct srcu_data access

2020-05-20 Thread Joel Fernandes
On Wed, May 20, 2020 at 08:35:29PM +0200, Peter Zijlstra wrote: > On Wed, May 20, 2020 at 08:28:00PM +0200, Sebastian Andrzej Siewior wrote: > > On 2020-05-20 13:42:59 [-0400], Joel Fernandes wrote: > > > Hi Sebastian, > > Hi Joel, > > > > > For pointer stability, can we just use get_local_ptr() a

Re: [PATCH 3/8] srcu: Use local_lock() for per-CPU struct srcu_data access

2020-05-20 Thread Paul E. McKenney
On Wed, May 20, 2020 at 02:06:08PM +0200, Sebastian Andrzej Siewior wrote: > On 2020-05-20 12:24:07 [+0200], Peter Zijlstra wrote: > > On Tue, May 19, 2020 at 10:19:07PM +0200, Sebastian Andrzej Siewior wrote: > > > > > diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c > > > index 0c71505

Re: [PATCH 3/8] srcu: Use local_lock() for per-CPU struct srcu_data access

2020-05-20 Thread Peter Zijlstra
On Wed, May 20, 2020 at 08:28:00PM +0200, Sebastian Andrzej Siewior wrote: > On 2020-05-20 13:42:59 [-0400], Joel Fernandes wrote: > > Hi Sebastian, > Hi Joel, > > > For pointer stability, can we just use get_local_ptr() and put_local_ptr() > > instead of adding an extra lock? This keeps the point

Re: [PATCH 3/8] srcu: Use local_lock() for per-CPU struct srcu_data access

2020-05-20 Thread Sebastian Andrzej Siewior
On 2020-05-20 13:42:59 [-0400], Joel Fernandes wrote: > Hi Sebastian, Hi Joel, > For pointer stability, can we just use get_local_ptr() and put_local_ptr() > instead of adding an extra lock? This keeps the pointer stable while keeping > the section preemptible on -rt. And we already have a lock in

Re: [PATCH 3/8] srcu: Use local_lock() for per-CPU struct srcu_data access

2020-05-20 Thread Joel Fernandes
Hi Sebastian, On Wed, May 20, 2020 at 02:06:08PM +0200, Sebastian Andrzej Siewior wrote: > On 2020-05-20 12:24:07 [+0200], Peter Zijlstra wrote: > > On Tue, May 19, 2020 at 10:19:07PM +0200, Sebastian Andrzej Siewior wrote: > > > > > diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c > >

Re: [PATCH 3/8] srcu: Use local_lock() for per-CPU struct srcu_data access

2020-05-20 Thread Peter Zijlstra
On Wed, May 20, 2020 at 02:06:08PM +0200, Sebastian Andrzej Siewior wrote: > On 2020-05-20 12:24:07 [+0200], Peter Zijlstra wrote: > > Would it perhaps make sense to stick the local_lock in struct srcu_data ? > > In that case we would need something for pointer stability before the > lock is acqu

Re: [PATCH 3/8] srcu: Use local_lock() for per-CPU struct srcu_data access

2020-05-20 Thread Sebastian Andrzej Siewior
On 2020-05-20 12:24:07 [+0200], Peter Zijlstra wrote: > On Tue, May 19, 2020 at 10:19:07PM +0200, Sebastian Andrzej Siewior wrote: > > > diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c > > index 0c71505f0e19c..8d2b5f75145d7 100644 > > --- a/kernel/rcu/srcutree.c > > +++ b/kernel/rcu/src

Re: [PATCH 3/8] srcu: Use local_lock() for per-CPU struct srcu_data access

2020-05-20 Thread Peter Zijlstra
On Tue, May 19, 2020 at 10:19:07PM +0200, Sebastian Andrzej Siewior wrote: > diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c > index 0c71505f0e19c..8d2b5f75145d7 100644 > --- a/kernel/rcu/srcutree.c > +++ b/kernel/rcu/srcutree.c > @@ -25,6 +25,7 @@ > #include > #include > #include

[PATCH 3/8] srcu: Use local_lock() for per-CPU struct srcu_data access

2020-05-19 Thread Sebastian Andrzej Siewior
SRCU disables interrupts to get a stable per-CPU pointer and then acquires the spinlock which is in the per-CPU data structure. The release uses spin_unlock_irqrestore(). While this is correct on a non-RT kernel, this conflicts with the RT semantics because the spinlock is converted to a 'sleeping'