| Message ID | 20231003232903.7109-3-frederic@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 530834c03e7f733712e24032adf83bddd3e99b83 |
| Headers | show |
| Series | srcu fixes | expand |
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);
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)); sdp->srcu_cblist_invoking = false; more = rcu_segcblist_ready_cbs(&sdp->srcu_cblist); spin_unlock_irq_rcu_node(sdp);
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(-)