Re: [PATCH 2/5] srcu: Only accelerate on enqueue time

Like Xu
2023-10-10T06:46:41Z

On 4/10/2023 7:29 am, Frederic Weisbecker wrote:
> Acceleration in SRCU happens on enqueue time for each new callback. This
> operation is expected not to fail and therefore any similar attempt
> from other places shouldn't find any remaining callbacks to accelerate.
>
> Moreover accelerations performed beyond enqueue time are error prone
> because rcu_seq_snap() then may return the snapshot for a new grace
> period that is not going to be started.
>
> Remove these dangerous and needless accelerations and introduce instead
> assertions reporting leaking unaccelerated callbacks beyond enqueue
> time.
>
> Co-developed-by: Yong He <zhuangel570@gmail.com>
> Co-developed-by: Joel Fernandes <joel@joelfernandes.org>
> Co-developed-by: Neeraj upadhyay <neeraj.iitr10@gmail.com>
> Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
> ---
> kernel/rcu/srcutree.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c
> index 9fab9ac36996..560e99ec5333 100644
> --- a/kernel/rcu/srcutree.c
> +++ b/kernel/rcu/srcutree.c
> @@ -784,8 +784,7 @@ static void srcu_gp_start(struct srcu_struct *ssp)
> spin_lock_rcu_node(sdp); /* Interrupts already disabled. */
> rcu_segcblist_advance(&sdp->srcu_cblist,
> rcu_seq_current(&ssp->srcu_sup->srcu_gp_seq));
> - (void)rcu_segcblist_accelerate(&sdp->srcu_cblist,
> - rcu_seq_snap(&ssp->srcu_sup->srcu_gp_seq));
> + WARN_ON_ONCE(!rcu_segcblist_segempty(&sdp->srcu_cblist, RCU_NEXT_TAIL));
> spin_unlock_rcu_node(sdp); /* Interrupts remain disabled. */
> WRITE_ONCE(ssp->srcu_sup->srcu_gp_start, jiffies);
> WRITE_ONCE(ssp->srcu_sup->srcu_n_exp_nodelay, 0);
> @@ -1721,6 +1720,7 @@ static void srcu_invoke_callbacks(struct work_struct *work)
> ssp = sdp->ssp;
> rcu_cblist_init(&ready_cbs);
> spin_lock_irq_rcu_node(sdp);
> + WARN_ON_ONCE(!rcu_segcblist_segempty(&sdp->srcu_cblist, RCU_NEXT_TAIL));
> rcu_segcblist_advance(&sdp->srcu_cblist,
> rcu_seq_current(&ssp->srcu_sup->srcu_gp_seq));
> if (sdp->srcu_cblist_invoking ||
> @@ -1750,8 +1750,6 @@ static void srcu_invoke_callbacks(struct work_struct *work)
> */
> spin_lock_irq_rcu_node(sdp);
> rcu_segcblist_add_len(&sdp->srcu_cblist, -len);
> - (void)rcu_segcblist_accelerate(&sdp->srcu_cblist,
> - rcu_seq_snap(&ssp->srcu_sup->srcu_gp_seq));

We did observe such issues at our farm and the same diff was applied,
thus this fix is especially appreciated. Thanks.

Reviewed-by: Like Xu <likexu@tencent.com>

> sdp->srcu_cblist_invoking = false;
> more = rcu_segcblist_ready_cbs(&sdp->srcu_cblist);
> spin_unlock_irq_rcu_node(sdp);