Re: [PATCH 1/3] rcu: Use static initializer for krc.lock

[Uladzislau Rezki <urezki@gmail.com>]

Sebastian,

> On 2020-04-16 17:01:43 [+0200], Uladzislau Rezki wrote:
> > > > @@ -3139,10 +3136,8 @@ void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
> > > >  	unsigned long flags;
> > > >  	struct kfree_rcu_cpu *krcp;
> > > >  
> > > > -	local_irq_save(flags);	// For safely calling this_cpu_ptr().
> > > > -	krcp = this_cpu_ptr(&krc);
> > > > -	if (krcp->initialized)
> > > > -		spin_lock(&krcp->lock);
> > > > +	krcp = raw_cpu_ptr(&krc);
> > > > +	spin_lock_irqsave(&krcp->lock, flags);
> > > 
> > It is not a good way to access to per-CPU variable. There is a race in
> > your code. So, we will rework it anyway soon. To guarantee that we stay
> > on the same CPU, first we disable IRQ's then we access per-CPU var and
> > take a spinlock.
> 
> What is the worst thing that can happen if a CPU migration happens
> between raw_cpu_ptr() and spin_lock_irqsave() ?
> 
You will just introduce higher rate of lock contention what we would
like to get rid of. Could you please explain what is broken when you
do like:

<snip>
local_irq_save(flags);
krcp = this_cpu_ptr(&krc);
spin_lock(&krcp->lock);
<snip>

Thanks!

--
Vlad Rezki