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

[Like Xu]

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@xxxxxxxxx>
> Co-developed-by: Joel Fernandes <joel@xxxxxxxxxxxxxxxxx>
> Co-developed-by: Neeraj upadhyay <neeraj.iitr10@xxxxxxxxx>
> Signed-off-by: Frederic Weisbecker <frederic@xxxxxxxxxx>
> ---
>   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@xxxxxxxxxxx>

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