Re: [PATCH] rcu/nocb: Extend checks for offloaded rdp by migrate_disable

[Frederic Weisbecker <frederic@kernel.org>]

On Tue, Jul 27, 2021 at 10:23:51AM -0700, Paul E. McKenney wrote:
> On Tue, Jul 27, 2021 at 06:38:15PM +0200, Sebastian Andrzej Siewior wrote:
> > Commit 3820b513a2e33 ("rcu/nocb: Detect unsafe checks for offloaded
> > rdp") added checks for safe rdp usage in the nocb case.
> > 
> > On PREEMPT_RT this checks triggers in the
> >  rcu_cpu_kthread() -> rcu_core() -> rcu_check_quiescent_state() ->
> >   rcu_rdp_is_offloaded()
> > 
> > call chain. On !PREEMPT_RT this warnings is suppressed because rcu_core()
> > is invoked with disabled BH which also disables preemption and
> > "preemptible()" is part of the checks which are considered safe.
> > On PREEMPT_RT disabling BH does not disable preemption and since there
> > is no other criteria the warning triggers.
> > 
> > According to the mentioned commit the goal is to check if the rcu data
> > pointer is "stable … and prevent from its value to be changed under us".
> > My understanding is that this may happen if the task is preemptible and
> > therefore free to be migrated to another CPU which would then lead to
> > another value of `rcu_data'.
> > One thing that has been overseen is that a task within a migrate-disable
> > region (as on PREEMPT_RT with disabled BH) is fully preemptible but may
> > not be migrated to another CPU which should be enough to guarantee that
> > rdp remains stable.
> > 
> > Check also disabled migration of the task if the RCU data pointer is
> > from current CPU. Put the whole check within an SMP ifdef block since
> > without SMP there are not CPU migrations to worry about (also
> > task_struct::migration_disabled is missing).
> > 
> > Cc: Frederic Weisbecker <frederic@kernel.org>
> > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > ---
> > I don't fully understand why the CPU-hotplug lock matters here but this
> > is beside the point ;)
> 
> If I remember correctly, any attempt to change the offloaded state
> must hold off CPU-hotplug operations.  So if the current thread is
> holding off CPU-hotplug operations, no other thread can be doing
> an offload or de-offload operation.

Exactly! :)