2022-06-20 22:22:38

by Paul E. McKenney

[permalink] [raw]
Subject: [PATCH rcu 03/12] rcu: Add rnp->cbovldmask check in rcutree_migrate_callbacks()

From: Zqiang <[email protected]>

Currently, the rcu_node structure's ->cbovlmask field is set in call_rcu()
when a given CPU is suffering from callback overload. But if that CPU
goes offline, the outgoing CPU's callbacks is migrated to the running
CPU, which is likely to overload the running CPU. However, that CPU's
bit in its leaf rcu_node structure's ->cbovlmask field remains zero.

Initially, this is OK because the outgoing CPU's bit remains set.
However, that bit will be cleared at the next end of a grace period,
at which time it is quite possible that the running CPU will still
be overloaded. If the running CPU invokes call_rcu(), then overload
will be checked for and the bit will be set. Except that there is no
guarantee that the running CPU will invoke call_rcu(), in which case the
next grace period will fail to take the running CPU's overload condition
into account. Plus, because the bit is not set, the end of the grace
period won't check for overload on this CPU.

This commit therefore adds a call to check_cb_ovld_locked() in
check_cb_ovld_locked() to set the running CPU's ->cbovlmask bit
appropriately.

Signed-off-by: Zqiang <[email protected]>
Signed-off-by: Paul E. McKenney <[email protected]>
---
kernel/rcu/tree.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index c19d5926886fb..f4a37f2032664 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -4491,6 +4491,7 @@ void rcutree_migrate_callbacks(int cpu)
needwake = needwake || rcu_advance_cbs(my_rnp, my_rdp);
rcu_segcblist_disable(&rdp->cblist);
WARN_ON_ONCE(rcu_segcblist_empty(&my_rdp->cblist) != !rcu_segcblist_n_cbs(&my_rdp->cblist));
+ check_cb_ovld_locked(my_rdp, my_rnp);
if (rcu_rdp_is_offloaded(my_rdp)) {
raw_spin_unlock_rcu_node(my_rnp); /* irqs remain disabled. */
__call_rcu_nocb_wake(my_rdp, true, flags);
--
2.31.1.189.g2e36527f23


2022-06-21 06:03:16

by Neeraj Upadhyay

[permalink] [raw]
Subject: Re: [PATCH rcu 03/12] rcu: Add rnp->cbovldmask check in rcutree_migrate_callbacks()



On 6/21/2022 3:50 AM, Paul E. McKenney wrote:
> From: Zqiang <[email protected]>
>
> Currently, the rcu_node structure's ->cbovlmask field is set in call_rcu()
> when a given CPU is suffering from callback overload. But if that CPU
> goes offline, the outgoing CPU's callbacks is migrated to the running
> CPU, which is likely to overload the running CPU. However, that CPU's
> bit in its leaf rcu_node structure's ->cbovlmask field remains zero.
>
> Initially, this is OK because the outgoing CPU's bit remains set.
> However, that bit will be cleared at the next end of a grace period,
> at which time it is quite possible that the running CPU will still
> be overloaded. If the running CPU invokes call_rcu(), then overload
> will be checked for and the bit will be set. Except that there is no
> guarantee that the running CPU will invoke call_rcu(), in which case the
> next grace period will fail to take the running CPU's overload condition
> into account. Plus, because the bit is not set, the end of the grace
> period won't check for overload on this CPU.
>
> This commit therefore adds a call to check_cb_ovld_locked() in
> check_cb_ovld_locked() to set the running CPU's ->cbovlmask bit

Nit: s/check_cb_ovld_locked/rcutree_migrate_callbacks/

> appropriately.
>
> Signed-off-by: Zqiang <[email protected]>
> Signed-off-by: Paul E. McKenney <[email protected]>
> ---

Reviewed-by: Neeraj Upadhyay <[email protected]>


Thanks
Neeraj

> kernel/rcu/tree.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index c19d5926886fb..f4a37f2032664 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -4491,6 +4491,7 @@ void rcutree_migrate_callbacks(int cpu)
> needwake = needwake || rcu_advance_cbs(my_rnp, my_rdp);
> rcu_segcblist_disable(&rdp->cblist);
> WARN_ON_ONCE(rcu_segcblist_empty(&my_rdp->cblist) != !rcu_segcblist_n_cbs(&my_rdp->cblist));
> + check_cb_ovld_locked(my_rdp, my_rnp);
> if (rcu_rdp_is_offloaded(my_rdp)) {
> raw_spin_unlock_rcu_node(my_rnp); /* irqs remain disabled. */
> __call_rcu_nocb_wake(my_rdp, true, flags);

2022-06-21 22:25:01

by Paul E. McKenney

[permalink] [raw]
Subject: Re: [PATCH rcu 03/12] rcu: Add rnp->cbovldmask check in rcutree_migrate_callbacks()

On Tue, Jun 21, 2022 at 11:27:28AM +0530, Neeraj Upadhyay wrote:
>
>
> On 6/21/2022 3:50 AM, Paul E. McKenney wrote:
> > From: Zqiang <[email protected]>
> >
> > Currently, the rcu_node structure's ->cbovlmask field is set in call_rcu()
> > when a given CPU is suffering from callback overload. But if that CPU
> > goes offline, the outgoing CPU's callbacks is migrated to the running
> > CPU, which is likely to overload the running CPU. However, that CPU's
> > bit in its leaf rcu_node structure's ->cbovlmask field remains zero.
> >
> > Initially, this is OK because the outgoing CPU's bit remains set.
> > However, that bit will be cleared at the next end of a grace period,
> > at which time it is quite possible that the running CPU will still
> > be overloaded. If the running CPU invokes call_rcu(), then overload
> > will be checked for and the bit will be set. Except that there is no
> > guarantee that the running CPU will invoke call_rcu(), in which case the
> > next grace period will fail to take the running CPU's overload condition
> > into account. Plus, because the bit is not set, the end of the grace
> > period won't check for overload on this CPU.
> >
> > This commit therefore adds a call to check_cb_ovld_locked() in
> > check_cb_ovld_locked() to set the running CPU's ->cbovlmask bit
>
> Nit: s/check_cb_ovld_locked/rcutree_migrate_callbacks/

Good catch, fixed!

> > appropriately.
> >
> > Signed-off-by: Zqiang <[email protected]>
> > Signed-off-by: Paul E. McKenney <[email protected]>
> > ---
>
> Reviewed-by: Neeraj Upadhyay <[email protected]>

Thank you, applied.

Thanx, Paul

> Thanks
> Neeraj
>
> > kernel/rcu/tree.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > index c19d5926886fb..f4a37f2032664 100644
> > --- a/kernel/rcu/tree.c
> > +++ b/kernel/rcu/tree.c
> > @@ -4491,6 +4491,7 @@ void rcutree_migrate_callbacks(int cpu)
> > needwake = needwake || rcu_advance_cbs(my_rnp, my_rdp);
> > rcu_segcblist_disable(&rdp->cblist);
> > WARN_ON_ONCE(rcu_segcblist_empty(&my_rdp->cblist) != !rcu_segcblist_n_cbs(&my_rdp->cblist));
> > + check_cb_ovld_locked(my_rdp, my_rnp);
> > if (rcu_rdp_is_offloaded(my_rdp)) {
> > raw_spin_unlock_rcu_node(my_rnp); /* irqs remain disabled. */
> > __call_rcu_nocb_wake(my_rdp, true, flags);