On Thu, Apr 16, 2020 at 02:41:12PM -0400, Joel Fernandes wrote: > On Thu, Apr 16, 2020 at 05:18:24PM +0200, Sebastian Andrzej Siewior wrote: > > On 2020-04-16 10:42:54 [-0400], Joel Fernandes wrote: > > > Hi Sebastian, > > Hi Joel, > > > > > > @@ -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); > > > > > > I agree with the patch, except for this bit. Would it be ok to split the > > > other parts of this patch into separate patch(es)? > > > > if you want, I could split it. Part of the goal is to get rid of the > > local_irq_save(). I don't mind if it happens a patch later :) > > Considering the other parts of your patch are less contentious, it makes > sense to me to split it, while we discuss this particular part ;) > > > > For this part of the patch, I am wondering what is the value in it. For one, > > > > local_irq_save() + spin_lock() is the problem, see > > https://www.kernel.org/doc/html/latest/locking/locktypes.html#spinlock-t-and-rwlock-t > > > > (the link-instead-explaining part is nice) > > This problem can be fixed simply by using raw_spin_lock above right? That > would solve the rtmutex problem in PREEMPT_RT. I guess a higher level question is, on PREEMPT_RT, do we want to make the kfree_rcu() code fully preemptible? I am not saying we should not, but that is another thing to consider. Obviously, the migrate_disable(); idea would be better then if are Ok with making it preemptible. But, then will it be safe for kfree_rcu() callers from hard IRQ context to call this in PREEMPT_RT? That could would just break then as you cannot sleep in hard IRQ context even on PREEMPT_RT. thanks, - Joel
