| Message ID | 20220606065143.3159643-1-qiang1.zhang@intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | rcu/nocb: Avoid polling when myrdp->nocb_head_rdp list is empty | expand |
On Mon, Jun 06, 2022 at 02:51:43PM +0800, Zqiang wrote: > Currently, If the 'rcu_nocb_poll' bootargs is enable, all rcuog kthreads > enter polling mode. however, due to only insert CPU's rdp which belong to > rcu_nocb_mask to 'nocb_head_rdp' list or all CPU's rdp served by rcuog > kthread have been de-offloaded, these cause the 'nocb_head_rdp' list > served by rcuog kthread is empty, when the 'nocb_head_rdp' is empty, > the rcuog kthread in polling mode not actually do anything. fix it by > exiting polling mode when the 'nocb_head_rdp'list is empty, otherwise > entering polling mode. > > Signed-off-by: Zqiang <qiang1.zhang@intel.com> Looks promising! A couple of questions below. > --- > kernel/rcu/tree_nocb.h | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h > index fa8e4f82e60c..c25c3e3dd22d 100644 > --- a/kernel/rcu/tree_nocb.h > +++ b/kernel/rcu/tree_nocb.h > @@ -698,10 +698,14 @@ static void nocb_gp_wait(struct rcu_data *my_rdp) > TPS("WakeBypassIsDeferred")); > } > if (rcu_nocb_poll) { > - /* Polling, so trace if first poll in the series. */ > - if (gotcbs) > - trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("Poll")); > - schedule_timeout_idle(1); > + if (list_empty(&my_rdp->nocb_head_rdp)) { Do we need to set rdp_gp->nocb_gp_sleep to true in here somewhere? > + rcu_wait(READ_ONCE(my_rdp->nocb_toggling_rdp)); > + } else { > + /* Polling, so trace if first poll in the series. */ > + if (gotcbs) > + trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("Poll")); > + schedule_timeout_idle(1); > + } > } else if (!needwait_gp) { > /* Wait for callbacks to appear. */ > trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("Sleep")); > @@ -980,7 +984,7 @@ static int rdp_offload_toggle(struct rcu_data *rdp, > raw_spin_lock_irqsave(&rdp_gp->nocb_gp_lock, flags); > // Queue this rdp for add/del to/from the list to iterate on rcuog > WRITE_ONCE(rdp_gp->nocb_toggling_rdp, rdp); > - if (rdp_gp->nocb_gp_sleep) { > + if (rcu_nocb_poll || rdp_gp->nocb_gp_sleep) { Is it the case that other than when (de-)offloading, the rcuog kthread is never awakened when rcu_nocb_poll is set? If this is not the case, we would have spurious wakeups. (I believe that rcu_nocb_poll suppresses those wakeups, but it would be good to check.) > rdp_gp->nocb_gp_sleep = false; > wake_gp = true; > } > -- > 2.25.1 >
On Mon, Jun 06, 2022 at 02:51:43PM +0800, Zqiang wrote: > Currently, If the 'rcu_nocb_poll' bootargs is enable, all rcuog kthreads > enter polling mode. however, due to only insert CPU's rdp which belong to > rcu_nocb_mask to 'nocb_head_rdp' list or all CPU's rdp served by rcuog > kthread have been de-offloaded, these cause the 'nocb_head_rdp' list > served by rcuog kthread is empty, when the 'nocb_head_rdp' is empty, > the rcuog kthread in polling mode not actually do anything. fix it by > exiting polling mode when the 'nocb_head_rdp'list is empty, otherwise > entering polling mode. > > Signed-off-by: Zqiang <qiang1.zhang@intel.com> >Looks promising! A couple of questions below. > --- > kernel/rcu/tree_nocb.h | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h > index fa8e4f82e60c..c25c3e3dd22d 100644 > --- a/kernel/rcu/tree_nocb.h > +++ b/kernel/rcu/tree_nocb.h > @@ -698,10 +698,14 @@ static void nocb_gp_wait(struct rcu_data *my_rdp) > TPS("WakeBypassIsDeferred")); > } > if (rcu_nocb_poll) { > - /* Polling, so trace if first poll in the series. */ > - if (gotcbs) > - trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("Poll")); > - schedule_timeout_idle(1); > + if (list_empty(&my_rdp->nocb_head_rdp)) { >Do we need to set rdp_gp->nocb_gp_sleep to true in here somewhere? When rcu_nocb_poll is set, the rdp_gp->nocb_gp_sleep flags is not used. > + rcu_wait(READ_ONCE(my_rdp->nocb_toggling_rdp)); > + } else { > + /* Polling, so trace if first poll in the series. */ > + if (gotcbs) > + trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("Poll")); > + schedule_timeout_idle(1); > + } > } else if (!needwait_gp) { > /* Wait for callbacks to appear. */ > trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("Sleep")); > @@ -980,7 +984,7 @@ static int rdp_offload_toggle(struct rcu_data *rdp, > raw_spin_lock_irqsave(&rdp_gp->nocb_gp_lock, flags); > // Queue this rdp for add/del to/from the list to iterate on rcuog > WRITE_ONCE(rdp_gp->nocb_toggling_rdp, rdp); > - if (rdp_gp->nocb_gp_sleep) { > + if (rcu_nocb_poll || rdp_gp->nocb_gp_sleep) { >Is it the case that other than when (de-)offloading, the rcuog kthread >is never awakened when rcu_nocb_poll is set? If this is not the case, >we would have spurious wakeups. (I believe that rcu_nocb_poll suppresses >those wakeups, but it would be good to check.) When rcu_nocb_poll is set, only when (de-)offloading, actively call wake_up_process(rdp_gp->nocb_gp_kthread). in other cases the __call_rcu_nocb_wake() will wakeup nocb_gp_kthread, but it met rcu_nocb_poll flags directly return. In do_nocb_deferred_wakeup(), although there is no rcu_nocb_poll check, but rdp->nocb_defer_wakeup is always RCU_NOCB_WAKE_NOT, also directly return. Due to rcu_nocb_poll is set, the rdp_gp->nocb_gp_sleep flags is not used, and Not should be set. move rcu_nocb_poll from rdp_offload_toggle() to rcu_nocb_rdp_offload/deoffload() Thanks Zqiang > rdp_gp->nocb_gp_sleep = false; > wake_gp = true; > } > -- > 2.25.1 >
diff --git a/kernel/rcu/tree_nocb.h b/kernel/rcu/tree_nocb.h index fa8e4f82e60c..c25c3e3dd22d 100644 --- a/kernel/rcu/tree_nocb.h +++ b/kernel/rcu/tree_nocb.h @@ -698,10 +698,14 @@ static void nocb_gp_wait(struct rcu_data *my_rdp) TPS("WakeBypassIsDeferred")); } if (rcu_nocb_poll) { - /* Polling, so trace if first poll in the series. */ - if (gotcbs) - trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("Poll")); - schedule_timeout_idle(1); + if (list_empty(&my_rdp->nocb_head_rdp)) { + rcu_wait(READ_ONCE(my_rdp->nocb_toggling_rdp)); + } else { + /* Polling, so trace if first poll in the series. */ + if (gotcbs) + trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("Poll")); + schedule_timeout_idle(1); + } } else if (!needwait_gp) { /* Wait for callbacks to appear. */ trace_rcu_nocb_wake(rcu_state.name, cpu, TPS("Sleep")); @@ -980,7 +984,7 @@ static int rdp_offload_toggle(struct rcu_data *rdp, raw_spin_lock_irqsave(&rdp_gp->nocb_gp_lock, flags); // Queue this rdp for add/del to/from the list to iterate on rcuog WRITE_ONCE(rdp_gp->nocb_toggling_rdp, rdp); - if (rdp_gp->nocb_gp_sleep) { + if (rcu_nocb_poll || rdp_gp->nocb_gp_sleep) { rdp_gp->nocb_gp_sleep = false; wake_gp = true; }
Currently, If the 'rcu_nocb_poll' bootargs is enable, all rcuog kthreads enter polling mode. however, due to only insert CPU's rdp which belong to rcu_nocb_mask to 'nocb_head_rdp' list or all CPU's rdp served by rcuog kthread have been de-offloaded, these cause the 'nocb_head_rdp' list served by rcuog kthread is empty, when the 'nocb_head_rdp' is empty, the rcuog kthread in polling mode not actually do anything. fix it by exiting polling mode when the 'nocb_head_rdp'list is empty, otherwise entering polling mode. Signed-off-by: Zqiang <qiang1.zhang@intel.com> --- kernel/rcu/tree_nocb.h | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)