On 28/07/21 18:04, Paul E. McKenney wrote: > On Thu, Jul 29, 2021 at 12:01:37AM +0200, Frederic Weisbecker wrote: >> On Wed, Jul 28, 2021 at 08:34:14PM +0100, Valentin Schneider wrote: >> > Now, if the offloaded state was (properly) protected by a local_lock, do >> > you reckon we could then keep preemption enabled? >> >> I guess we could take such a local lock on the update side >> (rcu_nocb_rdp_offload) and then take it on rcuc kthread/softirqs >> and maybe other places. >> >> But we must make sure that rcu_core() is preempt-safe from a general perspective >> in the first place. From a quick glance I can't find obvious issues...yet. >> >> Paul maybe you can see something? > > Let's see... > > o Extra context switches in rcu_core() mean extra quiescent > states. It therefore might be necessary to wrap rcu_core() > in an rcu_read_lock() / rcu_read_unlock() pair, because > otherwise an RCU grace period won't wait for rcu_core(). > > Actually, better have local_bh_disable() imply > rcu_read_lock() and local_bh_enable() imply rcu_read_unlock(). > But I would hope that this already happened. It does look like it. > > o The rcu_preempt_deferred_qs() check should still be fine, > unless there is a raw_bh_disable() in -rt. > > o The set_tsk_need_resched() and set_preempt_need_resched() > might preempt immediately. I cannot think of a problem > with that, but careful testing is clearly in order. > > o The values checked by rcu_check_quiescent_state() could now > change while this function is running. I don't immediately > see a problematic sequence of events, but here be dragons. > I therefore suggest disabling preemption across this function. > Or if that is impossible, taking a very careful look at the > proposed expansion of the state space of this function. > > o I don't see any new races in the grace-period/callback check. > New callbacks can appear in interrupt handlers, after all. > > o The rcu_check_gp_start_stall() function looks similarly > unproblematic. > > o Callback invocation can now be preempted, but then again it > recently started being concurrent, so this should be no > added risk over offloading/de-offloading. > > o I don't see any problem with do_nocb_deferred_wakeup(). > > o The CONFIG_RCU_STRICT_GRACE_PERIOD check should not be > impacted. > > So some adjustments might be needed, but I don't see a need for > major surgery. > > This of course might be a failure of imagination on my part, so it > wouldn't hurt to double-check my observations. > I'll go poke around, thank you both!
