Hi Boqun, On Thu, Oct 14, 2021 at 12:07:58AM +0800, Boqun Feng wrote: > If offloading races with rcu_core(), can the following happen? > > <offload work> > rcu_nocb_rdp_offload(): > rcu_core(): > ... > rcu_nocb_lock_irqsave(); // no a lock > raw_spin_lock_irqsave(->nocb_lock); > rdp_offload_toggle(): > <LOCKING | OFFLOADED set> > if (!rcu_segcblist_restempty(...)) > rcu_accelerate_cbs_unlocked(...); > rcu_nocb_unlock_irqrestore(); > // ^ a real unlock, > // and will preempt_enable() > // offload continue with ->nocb_lock not held > > If this can happen, it means an unpaired preempt_enable() and an > incorrect unlock. Thoughts? Maybe I'm missing something here? Since we are unconditionally disabling IRQs on rcu_nocb_lock_irqsave(), we can't be preempted by rcu_nocb_rdp_offload() until rcu_nocb_unlock_irqrestore() has completed. And both have to run on the rdp target CPU. So this shouldn't happen. Thanks. > > Regards, > Boqun > > > + * > > + * _ Deoffloading: In the worst case we miss callbacks acceleration or > > + * processing. This is fine because the early stage > > + * of deoffloading invokes rcu_core() after setting > > + * SEGCBLIST_RCU_CORE. So we guarantee that we'll process > > + * what could have been dismissed without the need to wait > > + * for the next rcu_pending() check in the next jiffy. > > + */ > > const bool do_batch = !rcu_segcblist_completely_offloaded(&rdp->cblist); > > > > if (cpu_is_offline(smp_processor_id())) > > diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h > > index 71a28f50b40f..3b470113ae38 100644 > > --- a/kernel/rcu/tree_nocb.h > > +++ b/kernel/rcu/tree_nocb.h > > @@ -990,6 +990,15 @@ static long rcu_nocb_rdp_deoffload(void *arg) > > * will refuse to put anything into the bypass. > > */ > > WARN_ON_ONCE(!rcu_nocb_flush_bypass(rdp, NULL, jiffies)); > > + /* > > + * Start with invoking rcu_core() early. This way if the current thread > > + * happens to preempt an ongoing call to rcu_core() in the middle, > > + * leaving some work dismissed because rcu_core() still thinks the rdp is > > + * completely offloaded, we are guaranteed a nearby future instance of > > + * rcu_core() to catch up. > > + */ > > + rcu_segcblist_set_flags(cblist, SEGCBLIST_RCU_CORE); > > + invoke_rcu_core(); > > ret = rdp_offload_toggle(rdp, false, flags); > > swait_event_exclusive(rdp->nocb_state_wq, > > !rcu_segcblist_test_flags(cblist, SEGCBLIST_KTHREAD_CB | > > -- > > 2.25.1 > >
