On Mon, Mar 11, 2024 at 11:55:02AM +0800, Zqiang wrote:
> This commit generate rcu_task_gp_in_progress() to check whether
> the rcu-tasks GP is ongoing, if is ongoing, avoid trigger
> rtp_irq_work to wakeup rcu tasks kthreads in call_rcu_tasks_generic().
>
> The test results are as follows:
>
> echo call_rcu_tasks_iw_wakeup > /sys/kernel/debug/tracing/set_ftrace_filter
> echo 1 > /sys/kernel/debug/tracing/function_profile_enabled
> insmod rcutorture.ko torture_type=tasks-tracing fwd_progress=4
> sleep 600
> rmmod rcutorture.ko
> echo 0 > /sys/kernel/debug/tracing/function_profile_enabled
> echo > /sys/kernel/debug/tracing/set_ftrace_filter
>
> head /sys/kernel/debug/tracing/trace_stat/function*
>
> original: 56376 apply patch: 33521
>
> Signed-off-by: Zqiang <[email protected]>
Note that rcu_seq_current() does not provide ordering. So are you
sure that this change is safe on weakly ordered systems such as ARM?
For example, consider the following sequence of events:
o The call_rcu_tasks_generic() function picks up the grace-period
sequence number, which shows that there is a grace period in
progress.
o The grace period ends, and sees no reason to start a new grace
period.
o The call_rcu_tasks_generic() function sees no reason to wake
up the grace-period kthread. There are no more calls to
call_rcu_tasks*(), so the callback is never invoked.
Or is there something that prevents this sequence of events from
ever happening on weakly ordered systems?
Thanx, Paul
> ---
>
> original:
> ==> /sys/kernel/debug/tracing/trace_stat/function0 <==
> Function Hit Time Avg
> s^2
> -------- --- ---- ---
> ---
> call_rcu_tasks_iw_wakeup 13217 19292.52 us 1.459 us
> 8.834 us
>
> ==> /sys/kernel/debug/tracing/trace_stat/function1 <==
> Function Hit Time Avg
> s^2
> -------- --- ---- ---
> ---
> call_rcu_tasks_iw_wakeup 15146 22377.01 us 1.477 us
> 22.873 us
>
> ==> /sys/kernel/debug/tracing/trace_stat/function2 <==
> Function Hit Time Avg
> s^2
> -------- --- ---- ---
> ---
> call_rcu_tasks_iw_wakeup 12561 18125.76 us 1.443 us
> 6.372 us
>
> ==> /sys/kernel/debug/tracing/trace_stat/function3 <==
> Function Hit Time Avg
> s^2
> -------- --- ---- ---
> ---
> call_rcu_tasks_iw_wakeup 15452 21770.57 us 1.408 us
> 6.710 us
>
> apply patch:
> ==> /sys/kernel/debug/tracing/trace_stat/function0 <==
> Function Hit Time Avg
> s^2
> -------- --- ---- ---
> ---
> call_rcu_tasks_iw_wakeup 8334 15121.13 us 1.814 us
> 4.457 us
>
> ==> /sys/kernel/debug/tracing/trace_stat/function1 <==
> Function Hit Time Avg
> s^2
> -------- --- ---- ---
> ---
> call_rcu_tasks_iw_wakeup 8355 15760.51 us 1.886 us
> 14.775 us
>
> ==> /sys/kernel/debug/tracing/trace_stat/function2 <==
> Function Hit Time Avg
> s^2
> -------- --- ---- ---
> ---
> call_rcu_tasks_iw_wakeup 7219 14194.27 us 1.966 us
> 42.440 us
>
> ==> /sys/kernel/debug/tracing/trace_stat/function3 <==
> Function Hit Time Avg
> s^2
> -------- --- ---- ---
> ---
> call_rcu_tasks_iw_wakeup 9613 19850.04 us 2.064 us
> 91.023 us
>
> kernel/rcu/tasks.h | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/rcu/tasks.h b/kernel/rcu/tasks.h
> index 147b5945d67a..36c7e1d441d0 100644
> --- a/kernel/rcu/tasks.h
> +++ b/kernel/rcu/tasks.h
> @@ -317,6 +317,11 @@ static void call_rcu_tasks_iw_wakeup(struct irq_work
> *iwp)
> rcuwait_wake_up(&rtp->cbs_wait);
> }
>
> +static int rcu_task_gp_in_progress(struct rcu_tasks *rtp)
> +{
> + return rcu_seq_state(rcu_seq_current(&rtp->tasks_gp_seq));
> +}
> +
> // Enqueue a callback for the specified flavor of Tasks RCU.
> static void call_rcu_tasks_generic(struct rcu_head *rhp, rcu_callback_t func,
> struct rcu_tasks *rtp)
> @@ -375,7 +380,8 @@ static void call_rcu_tasks_generic(struct rcu_head *rhp,
> rcu_callback_t func,
> }
> rcu_read_unlock();
> /* We can't create the thread unless interrupts are enabled. */
> - if (needwake && READ_ONCE(rtp->kthread_ptr))
> + if (needwake && READ_ONCE(rtp->kthread_ptr) &&
> + !rcu_task_gp_in_progress(rtp))
> irq_work_queue(&rtpcp->rtp_irq_work);
> }
>
> --
> 2.17.1
>
