Re: [RFC PATCH 07/12] rcu: Shutdown nocb timer on de-offloading
On Wed, Sep 23, 2020 at 05:29:52PM +0200, Frederic Weisbecker wrote: > On Mon, Sep 21, 2020 at 05:17:56PM -0700, Paul E. McKenney wrote: > > On Mon, Sep 21, 2020 at 02:43:46PM +0200, Frederic Weisbecker wrote: > > > Make sure the nocb timer can't fire anymore before we reach the final > > > de-offload state. Spuriously waking up the GP kthread is no big deal but > > > we must prevent from executing the timer callback without nocb locking. > > > > If we had just the previous patch and not this patch, would things break? > > Or are you relying on the fact that there is not yet a connection to > > userspace controls for this functionality? > > Exactly it shouldn't break because only the last patch makes the code > eventually used. That works, thank you! Thanx, Paul
Re: [RFC PATCH 07/12] rcu: Shutdown nocb timer on de-offloading
On Mon, Sep 21, 2020 at 05:17:56PM -0700, Paul E. McKenney wrote: > On Mon, Sep 21, 2020 at 02:43:46PM +0200, Frederic Weisbecker wrote: > > Make sure the nocb timer can't fire anymore before we reach the final > > de-offload state. Spuriously waking up the GP kthread is no big deal but > > we must prevent from executing the timer callback without nocb locking. > > If we had just the previous patch and not this patch, would things break? > Or are you relying on the fact that there is not yet a connection to > userspace controls for this functionality? Exactly it shouldn't break because only the last patch makes the code eventually used. Thanks.
Re: [RFC PATCH 07/12] rcu: Shutdown nocb timer on de-offloading
On Mon, Sep 21, 2020 at 02:43:46PM +0200, Frederic Weisbecker wrote:
> Make sure the nocb timer can't fire anymore before we reach the final
> de-offload state. Spuriously waking up the GP kthread is no big deal but
> we must prevent from executing the timer callback without nocb locking.
If we had just the previous patch and not this patch, would things break?
Or are you relying on the fact that there is not yet a connection to
userspace controls for this functionality?
Just looking out for bisectability...
Thanx, Paul
> Inspired-by: Paul E. McKenney
> Signed-off-by: Frederic Weisbecker
> Cc: Paul E. McKenney
> Cc: Josh Triplett
> Cc: Steven Rostedt
> Cc: Mathieu Desnoyers
> Cc: Lai Jiangshan
> Cc: Joel Fernandes
> ---
> kernel/rcu/tree.h| 1 +
> kernel/rcu/tree_plugin.h | 12 +++-
> 2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
> index 5b37f7103b0d..ca69238e2227 100644
> --- a/kernel/rcu/tree.h
> +++ b/kernel/rcu/tree.h
> @@ -254,6 +254,7 @@ struct rcu_data {
> };
>
> /* Values for nocb_defer_wakeup field in struct rcu_data. */
> +#define RCU_NOCB_WAKE_OFF-1
> #define RCU_NOCB_WAKE_NOT0
> #define RCU_NOCB_WAKE1
> #define RCU_NOCB_WAKE_FORCE 2
> diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
> index eceef6dade9a..3361bd351f3b 100644
> --- a/kernel/rcu/tree_plugin.h
> +++ b/kernel/rcu/tree_plugin.h
> @@ -1637,6 +1637,8 @@ static void wake_nocb_gp(struct rcu_data *rdp, bool
> force,
> static void wake_nocb_gp_defer(struct rcu_data *rdp, int waketype,
> const char *reason)
> {
> + if (rdp->nocb_defer_wakeup == RCU_NOCB_WAKE_OFF)
> + return;
> if (rdp->nocb_defer_wakeup == RCU_NOCB_WAKE_NOT)
> mod_timer(&rdp->nocb_timer, jiffies + 1);
> if (rdp->nocb_defer_wakeup < waketype)
> @@ -2214,7 +2216,7 @@ static int rcu_nocb_cb_kthread(void *arg)
> /* Is a deferred wakeup of rcu_nocb_kthread() required? */
> static int rcu_nocb_need_deferred_wakeup(struct rcu_data *rdp)
> {
> - return READ_ONCE(rdp->nocb_defer_wakeup);
> + return READ_ONCE(rdp->nocb_defer_wakeup) > RCU_NOCB_WAKE_NOT;
> }
>
> /* Do a deferred wakeup of rcu_nocb_kthread(). */
> @@ -2299,6 +2301,12 @@ static void __rcu_nocb_rdp_deoffload(struct rcu_data
> *rdp)
> swait_event_exclusive(rdp->nocb_state_wq,
> !rcu_segcblist_test_flags(cblist,
> SEGCBLIST_KTHREAD_CB |
> SEGCBLIST_KTHREAD_GP));
> +
> + /* Make sure nocb timer won't stay around */
> + rcu_nocb_lock_irqsave(rdp, flags);
> + WRITE_ONCE(rdp->nocb_defer_wakeup, RCU_NOCB_WAKE_OFF);
> + rcu_nocb_unlock_irqrestore(rdp, flags);
> + del_timer_sync(&rdp->nocb_timer);
> }
>
> static long rcu_nocb_rdp_deoffload(void *arg)
> @@ -2344,6 +2352,8 @@ static void __rcu_nocb_rdp_offload(struct rcu_data *rdp)
>* SEGCBLIST_SOFTIRQ_ONLY mode.
>*/
> raw_spin_lock_irqsave(&rdp->nocb_lock, flags);
> + /* Re-enable nocb timer */
> + WRITE_ONCE(rdp->nocb_defer_wakeup, RCU_NOCB_WAKE_NOT);
> /*
>* We didn't take the nocb lock while working on the
>* rdp->cblist in SEGCBLIST_SOFTIRQ_ONLY mode.
> --
> 2.28.0
>
[RFC PATCH 07/12] rcu: Shutdown nocb timer on de-offloading
Make sure the nocb timer can't fire anymore before we reach the final
de-offload state. Spuriously waking up the GP kthread is no big deal but
we must prevent from executing the timer callback without nocb locking.
Inspired-by: Paul E. McKenney
Signed-off-by: Frederic Weisbecker
Cc: Paul E. McKenney
Cc: Josh Triplett
Cc: Steven Rostedt
Cc: Mathieu Desnoyers
Cc: Lai Jiangshan
Cc: Joel Fernandes
---
kernel/rcu/tree.h| 1 +
kernel/rcu/tree_plugin.h | 12 +++-
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
index 5b37f7103b0d..ca69238e2227 100644
--- a/kernel/rcu/tree.h
+++ b/kernel/rcu/tree.h
@@ -254,6 +254,7 @@ struct rcu_data {
};
/* Values for nocb_defer_wakeup field in struct rcu_data. */
+#define RCU_NOCB_WAKE_OFF -1
#define RCU_NOCB_WAKE_NOT 0
#define RCU_NOCB_WAKE 1
#define RCU_NOCB_WAKE_FORCE2
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index eceef6dade9a..3361bd351f3b 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -1637,6 +1637,8 @@ static void wake_nocb_gp(struct rcu_data *rdp, bool force,
static void wake_nocb_gp_defer(struct rcu_data *rdp, int waketype,
const char *reason)
{
+ if (rdp->nocb_defer_wakeup == RCU_NOCB_WAKE_OFF)
+ return;
if (rdp->nocb_defer_wakeup == RCU_NOCB_WAKE_NOT)
mod_timer(&rdp->nocb_timer, jiffies + 1);
if (rdp->nocb_defer_wakeup < waketype)
@@ -2214,7 +2216,7 @@ static int rcu_nocb_cb_kthread(void *arg)
/* Is a deferred wakeup of rcu_nocb_kthread() required? */
static int rcu_nocb_need_deferred_wakeup(struct rcu_data *rdp)
{
- return READ_ONCE(rdp->nocb_defer_wakeup);
+ return READ_ONCE(rdp->nocb_defer_wakeup) > RCU_NOCB_WAKE_NOT;
}
/* Do a deferred wakeup of rcu_nocb_kthread(). */
@@ -2299,6 +2301,12 @@ static void __rcu_nocb_rdp_deoffload(struct rcu_data
*rdp)
swait_event_exclusive(rdp->nocb_state_wq,
!rcu_segcblist_test_flags(cblist,
SEGCBLIST_KTHREAD_CB |
SEGCBLIST_KTHREAD_GP));
+
+ /* Make sure nocb timer won't stay around */
+ rcu_nocb_lock_irqsave(rdp, flags);
+ WRITE_ONCE(rdp->nocb_defer_wakeup, RCU_NOCB_WAKE_OFF);
+ rcu_nocb_unlock_irqrestore(rdp, flags);
+ del_timer_sync(&rdp->nocb_timer);
}
static long rcu_nocb_rdp_deoffload(void *arg)
@@ -2344,6 +2352,8 @@ static void __rcu_nocb_rdp_offload(struct rcu_data *rdp)
* SEGCBLIST_SOFTIRQ_ONLY mode.
*/
raw_spin_lock_irqsave(&rdp->nocb_lock, flags);
+ /* Re-enable nocb timer */
+ WRITE_ONCE(rdp->nocb_defer_wakeup, RCU_NOCB_WAKE_NOT);
/*
* We didn't take the nocb lock while working on the
* rdp->cblist in SEGCBLIST_SOFTIRQ_ONLY mode.
--
2.28.0
