Re: [PATCH v2] tracing/osnoise: Force quiescent states while tracing

[Daniel Bristot de Oliveira <bristot@kernel.org>]

On 3/7/22 19:07, Nicolas Saenz Julienne wrote:
> At the moment running osnoise on a nohz_full CPU or uncontested FIFO
> priority and a PREEMPT_RCU kernel might have the side effect of
> extending grace periods too much. This will entice RCU to force a
> context switch on the wayward CPU to end the grace period, all while
> introducing unwarranted noise into the tracer. This behaviour is
> unavoidable as overly extending grace periods might exhaust the system's
> memory.
> 
> This same exact problem is what extended quiescent states (EQS) were
> created for, conversely, rcu_momentary_dyntick_idle() emulates them by
> performing a zero duration EQS. So let's make use of it.
> 
> In the common case rcu_momentary_dyntick_idle() is fairly inexpensive:
> atomically incrementing a local per-CPU counter and doing a store. So it
> shouldn't affect osnoise's measurements (which has a 1us granularity),
> so we'll call it unanimously.
> 
> The uncommon case involve calling rcu_momentary_dyntick_idle() after
> having the osnoise process:
> 
>  - Receive an expedited quiescent state IPI with preemption disabled or
>    during an RCU critical section. (activates rdp->cpu_no_qs.b.exp
>    code-path).
> 
>  - Being preempted within in an RCU critical section and having the
>    subsequent outermost rcu_read_unlock() called with interrupts
>    disabled. (t->rcu_read_unlock_special.b.blocked code-path).
> 
> Neither of those are possible at the moment, and are unlikely to be in
> the future given the osnoise's loop design. On top of this, the noise
> generated by the situations described above is unavoidable, and if not
> exposed by rcu_momentary_dyntick_idle() will be eventually seen in
> subsequent rcu_read_unlock() calls or schedule operations.
> 
> Fixes: bce29ac9ce0b ("trace: Add osnoise tracer")
> Signed-off-by: Nicolas Saenz Julienne <nsaenzju@redhat.com>
> Acked-by: Paul E. McKenney <paulmck@kernel.org>

I've been running a preliminary version of this patch over the weekend, and it
provides the desired behavior.

Acked-by: Daniel Bristot de Oliveira <bristot@kernel.org>

Thanks!
-- Daniel