Re: [PATCH tip/core/rcu 04/12] rcu: Move expedited code from tree.c to tree_exp.h

[Paul E. McKenney]

On Fri, Jun 17, 2016 at 11:48:12AM -0400, Pranith Kumar wrote:
> Hi Paul,
> 
> On Wed, Jun 15, 2016 at 5:46 PM, Paul E. McKenney
>  wrote:
> > People have been having some difficulty finding their way around the
> > RCU code.  This commit therefore pulls some of the expedited grace-period
> > code from tree.c to a new tree_exp.h file.  This commit is strictly code
> > movement, with the exception of a forward declaration that was added
> > for the sync_sched_exp_online_cleanup() function.
> >
> > A subsequent commit will move the remaining expedited grace-period code
> > from tree_plugin.h to tree_exp.h.
> >
> > Signed-off-by: Paul E. McKenney 
> > ---
> >  kernel/rcu/tree.c | 545 
> > +---
> >  kernel/rcu/tree_exp.h | 564 
> > ++
> >  2 files changed, 566 insertions(+), 543 deletions(-)
> >  create mode 100644 kernel/rcu/tree_exp.h
> >
> 
> I've always wondered why you chose to only have a header file instead
> of the traditional x.h/x.c split for declarations and
> definitions(looking at tree_plugin.h). Is there any particular reason
> for this?

I didn't want to worry about function-call overhead, and doing it this
way allowed inlining.  Perhaps if link-time optimizations end up being
used heavily, I should go to the usual .h/.c split.

Thanx, Paul

Re: [PATCH tip/core/rcu 04/12] rcu: Move expedited code from tree.c to tree_exp.h

[Paul E. McKenney]

On Fri, Jun 17, 2016 at 11:48:12AM -0400, Pranith Kumar wrote:
> Hi Paul,
> 
> On Wed, Jun 15, 2016 at 5:46 PM, Paul E. McKenney
>  wrote:
> > People have been having some difficulty finding their way around the
> > RCU code.  This commit therefore pulls some of the expedited grace-period
> > code from tree.c to a new tree_exp.h file.  This commit is strictly code
> > movement, with the exception of a forward declaration that was added
> > for the sync_sched_exp_online_cleanup() function.
> >
> > A subsequent commit will move the remaining expedited grace-period code
> > from tree_plugin.h to tree_exp.h.
> >
> > Signed-off-by: Paul E. McKenney 
> > ---
> >  kernel/rcu/tree.c | 545 
> > +---
> >  kernel/rcu/tree_exp.h | 564 
> > ++
> >  2 files changed, 566 insertions(+), 543 deletions(-)
> >  create mode 100644 kernel/rcu/tree_exp.h
> >
> 
> I've always wondered why you chose to only have a header file instead
> of the traditional x.h/x.c split for declarations and
> definitions(looking at tree_plugin.h). Is there any particular reason
> for this?

I didn't want to worry about function-call overhead, and doing it this
way allowed inlining.  Perhaps if link-time optimizations end up being
used heavily, I should go to the usual .h/.c split.

Thanx, Paul

Re: [PATCH tip/core/rcu 04/12] rcu: Move expedited code from tree.c to tree_exp.h

[Pranith Kumar]

Hi Paul,

On Wed, Jun 15, 2016 at 5:46 PM, Paul E. McKenney
 wrote:
> People have been having some difficulty finding their way around the
> RCU code.  This commit therefore pulls some of the expedited grace-period
> code from tree.c to a new tree_exp.h file.  This commit is strictly code
> movement, with the exception of a forward declaration that was added
> for the sync_sched_exp_online_cleanup() function.
>
> A subsequent commit will move the remaining expedited grace-period code
> from tree_plugin.h to tree_exp.h.
>
> Signed-off-by: Paul E. McKenney 
> ---
>  kernel/rcu/tree.c | 545 +---
>  kernel/rcu/tree_exp.h | 564 
> ++
>  2 files changed, 566 insertions(+), 543 deletions(-)
>  create mode 100644 kernel/rcu/tree_exp.h
>

I've always wondered why you chose to only have a header file instead
of the traditional x.h/x.c split for declarations and
definitions(looking at tree_plugin.h). Is there any particular reason
for this?

Thanks,
-- 
Pranith

Re: [PATCH tip/core/rcu 04/12] rcu: Move expedited code from tree.c to tree_exp.h

[Pranith Kumar]

Hi Paul,

On Wed, Jun 15, 2016 at 5:46 PM, Paul E. McKenney
 wrote:
> People have been having some difficulty finding their way around the
> RCU code.  This commit therefore pulls some of the expedited grace-period
> code from tree.c to a new tree_exp.h file.  This commit is strictly code
> movement, with the exception of a forward declaration that was added
> for the sync_sched_exp_online_cleanup() function.
>
> A subsequent commit will move the remaining expedited grace-period code
> from tree_plugin.h to tree_exp.h.
>
> Signed-off-by: Paul E. McKenney 
> ---
>  kernel/rcu/tree.c | 545 +---
>  kernel/rcu/tree_exp.h | 564 
> ++
>  2 files changed, 566 insertions(+), 543 deletions(-)
>  create mode 100644 kernel/rcu/tree_exp.h
>

I've always wondered why you chose to only have a header file instead
of the traditional x.h/x.c split for declarations and
definitions(looking at tree_plugin.h). Is there any particular reason
for this?

Thanks,
-- 
Pranith

Re: [PATCH tip/core/rcu 04/12] rcu: Move expedited code from tree.c to tree_exp.h

[Paul E. McKenney]

On Thu, Jun 16, 2016 at 12:05:39AM +0200, Peter Zijlstra wrote:
> On Wed, Jun 15, 2016 at 02:46:05PM -0700, Paul E. McKenney wrote:
> > People have been having some difficulty finding their way around the
> > RCU code.  This commit therefore pulls some of the expedited grace-period
> > code from tree.c to a new tree_exp.h file.  This commit is strictly code
> > movement, with the exception of a forward declaration that was added
> > for the sync_sched_exp_online_cleanup() function.
> > 
> > A subsequent commit will move the remaining expedited grace-period code
> > from tree_plugin.h to tree_exp.h.
> 
> Part of the problem for me is always the fact that its so weirdly
> divided over these files. Now you're making that worse. How is this
> helping?

The expedited stuff is now in one place rather than being split across
two different files.

Thanx, Paul

Re: [PATCH tip/core/rcu 04/12] rcu: Move expedited code from tree.c to tree_exp.h

[Paul E. McKenney]

On Thu, Jun 16, 2016 at 12:05:39AM +0200, Peter Zijlstra wrote:
> On Wed, Jun 15, 2016 at 02:46:05PM -0700, Paul E. McKenney wrote:
> > People have been having some difficulty finding their way around the
> > RCU code.  This commit therefore pulls some of the expedited grace-period
> > code from tree.c to a new tree_exp.h file.  This commit is strictly code
> > movement, with the exception of a forward declaration that was added
> > for the sync_sched_exp_online_cleanup() function.
> > 
> > A subsequent commit will move the remaining expedited grace-period code
> > from tree_plugin.h to tree_exp.h.
> 
> Part of the problem for me is always the fact that its so weirdly
> divided over these files. Now you're making that worse. How is this
> helping?

The expedited stuff is now in one place rather than being split across
two different files.

Thanx, Paul

Re: [PATCH tip/core/rcu 04/12] rcu: Move expedited code from tree.c to tree_exp.h

[Peter Zijlstra]

On Wed, Jun 15, 2016 at 02:46:05PM -0700, Paul E. McKenney wrote:
> People have been having some difficulty finding their way around the
> RCU code.  This commit therefore pulls some of the expedited grace-period
> code from tree.c to a new tree_exp.h file.  This commit is strictly code
> movement, with the exception of a forward declaration that was added
> for the sync_sched_exp_online_cleanup() function.
> 
> A subsequent commit will move the remaining expedited grace-period code
> from tree_plugin.h to tree_exp.h.

Part of the problem for me is always the fact that its so weirdly
divided over these files. Now you're making that worse. How is this
helping?

Re: [PATCH tip/core/rcu 04/12] rcu: Move expedited code from tree.c to tree_exp.h

[Peter Zijlstra]

On Wed, Jun 15, 2016 at 02:46:05PM -0700, Paul E. McKenney wrote:
> People have been having some difficulty finding their way around the
> RCU code.  This commit therefore pulls some of the expedited grace-period
> code from tree.c to a new tree_exp.h file.  This commit is strictly code
> movement, with the exception of a forward declaration that was added
> for the sync_sched_exp_online_cleanup() function.
> 
> A subsequent commit will move the remaining expedited grace-period code
> from tree_plugin.h to tree_exp.h.

Part of the problem for me is always the fact that its so weirdly
divided over these files. Now you're making that worse. How is this
helping?

[PATCH tip/core/rcu 04/12] rcu: Move expedited code from tree.c to tree_exp.h

[Paul E. McKenney]

People have been having some difficulty finding their way around the
RCU code.  This commit therefore pulls some of the expedited grace-period
code from tree.c to a new tree_exp.h file.  This commit is strictly code
movement, with the exception of a forward declaration that was added
for the sync_sched_exp_online_cleanup() function.

A subsequent commit will move the remaining expedited grace-period code
from tree_plugin.h to tree_exp.h.

Signed-off-by: Paul E. McKenney 
---
 kernel/rcu/tree.c | 545 +---
 kernel/rcu/tree_exp.h | 564 ++
 2 files changed, 566 insertions(+), 543 deletions(-)
 create mode 100644 kernel/rcu/tree_exp.h

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 4aefeafb9a95..c844b6142a86 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -159,6 +159,7 @@ static void invoke_rcu_core(void);
 static void invoke_rcu_callbacks(struct rcu_state *rsp, struct rcu_data *rdp);
 static void rcu_report_exp_rdp(struct rcu_state *rsp,
   struct rcu_data *rdp, bool wake);
+static void sync_sched_exp_online_cleanup(int cpu);
 
 /* rcuc/rcub kthread realtime priority */
 #ifdef CONFIG_RCU_KTHREAD_PRIO
@@ -3447,549 +3448,6 @@ static bool rcu_seq_done(unsigned long *sp, unsigned 
long s)
return ULONG_CMP_GE(READ_ONCE(*sp), s);
 }
 
-/* Wrapper functions for expedited grace periods.  */
-static void rcu_exp_gp_seq_start(struct rcu_state *rsp)
-{
-   rcu_seq_start(>expedited_sequence);
-}
-static void rcu_exp_gp_seq_end(struct rcu_state *rsp)
-{
-   rcu_seq_end(>expedited_sequence);
-   smp_mb(); /* Ensure that consecutive grace periods serialize. */
-}
-static unsigned long rcu_exp_gp_seq_snap(struct rcu_state *rsp)
-{
-   unsigned long s;
-
-   smp_mb(); /* Caller's modifications seen first by other CPUs. */
-   s = rcu_seq_snap(>expedited_sequence);
-   trace_rcu_exp_grace_period(rsp->name, s, TPS("snap"));
-   return s;
-}
-static bool rcu_exp_gp_seq_done(struct rcu_state *rsp, unsigned long s)
-{
-   return rcu_seq_done(>expedited_sequence, s);
-}
-
-/*
- * Reset the ->expmaskinit values in the rcu_node tree to reflect any
- * recent CPU-online activity.  Note that these masks are not cleared
- * when CPUs go offline, so they reflect the union of all CPUs that have
- * ever been online.  This means that this function normally takes its
- * no-work-to-do fastpath.
- */
-static void sync_exp_reset_tree_hotplug(struct rcu_state *rsp)
-{
-   bool done;
-   unsigned long flags;
-   unsigned long mask;
-   unsigned long oldmask;
-   int ncpus = READ_ONCE(rsp->ncpus);
-   struct rcu_node *rnp;
-   struct rcu_node *rnp_up;
-
-   /* If no new CPUs onlined since last time, nothing to do. */
-   if (likely(ncpus == rsp->ncpus_snap))
-   return;
-   rsp->ncpus_snap = ncpus;
-
-   /*
-* Each pass through the following loop propagates newly onlined
-* CPUs for the current rcu_node structure up the rcu_node tree.
-*/
-   rcu_for_each_leaf_node(rsp, rnp) {
-   raw_spin_lock_irqsave_rcu_node(rnp, flags);
-   if (rnp->expmaskinit == rnp->expmaskinitnext) {
-   raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
-   continue;  /* No new CPUs, nothing to do. */
-   }
-
-   /* Update this node's mask, track old value for propagation. */
-   oldmask = rnp->expmaskinit;
-   rnp->expmaskinit = rnp->expmaskinitnext;
-   raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
-
-   /* If was already nonzero, nothing to propagate. */
-   if (oldmask)
-   continue;
-
-   /* Propagate the new CPU up the tree. */
-   mask = rnp->grpmask;
-   rnp_up = rnp->parent;
-   done = false;
-   while (rnp_up) {
-   raw_spin_lock_irqsave_rcu_node(rnp_up, flags);
-   if (rnp_up->expmaskinit)
-   done = true;
-   rnp_up->expmaskinit |= mask;
-   raw_spin_unlock_irqrestore_rcu_node(rnp_up, flags);
-   if (done)
-   break;
-   mask = rnp_up->grpmask;
-   rnp_up = rnp_up->parent;
-   }
-   }
-}
-
-/*
- * Reset the ->expmask values in the rcu_node tree in preparation for
- * a new expedited grace period.
- */
-static void __maybe_unused sync_exp_reset_tree(struct rcu_state *rsp)
-{
-   unsigned long flags;
-   struct rcu_node *rnp;
-
-   sync_exp_reset_tree_hotplug(rsp);
-   rcu_for_each_node_breadth_first(rsp, rnp) {
-   raw_spin_lock_irqsave_rcu_node(rnp, flags);
-   WARN_ON_ONCE(rnp->expmask);
-  

[PATCH tip/core/rcu 04/12] rcu: Move expedited code from tree.c to tree_exp.h

[Paul E. McKenney]

People have been having some difficulty finding their way around the
RCU code.  This commit therefore pulls some of the expedited grace-period
code from tree.c to a new tree_exp.h file.  This commit is strictly code
movement, with the exception of a forward declaration that was added
for the sync_sched_exp_online_cleanup() function.

A subsequent commit will move the remaining expedited grace-period code
from tree_plugin.h to tree_exp.h.

Signed-off-by: Paul E. McKenney 
---
 kernel/rcu/tree.c | 545 +---
 kernel/rcu/tree_exp.h | 564 ++
 2 files changed, 566 insertions(+), 543 deletions(-)
 create mode 100644 kernel/rcu/tree_exp.h

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 4aefeafb9a95..c844b6142a86 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -159,6 +159,7 @@ static void invoke_rcu_core(void);
 static void invoke_rcu_callbacks(struct rcu_state *rsp, struct rcu_data *rdp);
 static void rcu_report_exp_rdp(struct rcu_state *rsp,
   struct rcu_data *rdp, bool wake);
+static void sync_sched_exp_online_cleanup(int cpu);
 
 /* rcuc/rcub kthread realtime priority */
 #ifdef CONFIG_RCU_KTHREAD_PRIO
@@ -3447,549 +3448,6 @@ static bool rcu_seq_done(unsigned long *sp, unsigned 
long s)
return ULONG_CMP_GE(READ_ONCE(*sp), s);
 }
 
-/* Wrapper functions for expedited grace periods.  */
-static void rcu_exp_gp_seq_start(struct rcu_state *rsp)
-{
-   rcu_seq_start(>expedited_sequence);
-}
-static void rcu_exp_gp_seq_end(struct rcu_state *rsp)
-{
-   rcu_seq_end(>expedited_sequence);
-   smp_mb(); /* Ensure that consecutive grace periods serialize. */
-}
-static unsigned long rcu_exp_gp_seq_snap(struct rcu_state *rsp)
-{
-   unsigned long s;
-
-   smp_mb(); /* Caller's modifications seen first by other CPUs. */
-   s = rcu_seq_snap(>expedited_sequence);
-   trace_rcu_exp_grace_period(rsp->name, s, TPS("snap"));
-   return s;
-}
-static bool rcu_exp_gp_seq_done(struct rcu_state *rsp, unsigned long s)
-{
-   return rcu_seq_done(>expedited_sequence, s);
-}
-
-/*
- * Reset the ->expmaskinit values in the rcu_node tree to reflect any
- * recent CPU-online activity.  Note that these masks are not cleared
- * when CPUs go offline, so they reflect the union of all CPUs that have
- * ever been online.  This means that this function normally takes its
- * no-work-to-do fastpath.
- */
-static void sync_exp_reset_tree_hotplug(struct rcu_state *rsp)
-{
-   bool done;
-   unsigned long flags;
-   unsigned long mask;
-   unsigned long oldmask;
-   int ncpus = READ_ONCE(rsp->ncpus);
-   struct rcu_node *rnp;
-   struct rcu_node *rnp_up;
-
-   /* If no new CPUs onlined since last time, nothing to do. */
-   if (likely(ncpus == rsp->ncpus_snap))
-   return;
-   rsp->ncpus_snap = ncpus;
-
-   /*
-* Each pass through the following loop propagates newly onlined
-* CPUs for the current rcu_node structure up the rcu_node tree.
-*/
-   rcu_for_each_leaf_node(rsp, rnp) {
-   raw_spin_lock_irqsave_rcu_node(rnp, flags);
-   if (rnp->expmaskinit == rnp->expmaskinitnext) {
-   raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
-   continue;  /* No new CPUs, nothing to do. */
-   }
-
-   /* Update this node's mask, track old value for propagation. */
-   oldmask = rnp->expmaskinit;
-   rnp->expmaskinit = rnp->expmaskinitnext;
-   raw_spin_unlock_irqrestore_rcu_node(rnp, flags);
-
-   /* If was already nonzero, nothing to propagate. */
-   if (oldmask)
-   continue;
-
-   /* Propagate the new CPU up the tree. */
-   mask = rnp->grpmask;
-   rnp_up = rnp->parent;
-   done = false;
-   while (rnp_up) {
-   raw_spin_lock_irqsave_rcu_node(rnp_up, flags);
-   if (rnp_up->expmaskinit)
-   done = true;
-   rnp_up->expmaskinit |= mask;
-   raw_spin_unlock_irqrestore_rcu_node(rnp_up, flags);
-   if (done)
-   break;
-   mask = rnp_up->grpmask;
-   rnp_up = rnp_up->parent;
-   }
-   }
-}
-
-/*
- * Reset the ->expmask values in the rcu_node tree in preparation for
- * a new expedited grace period.
- */
-static void __maybe_unused sync_exp_reset_tree(struct rcu_state *rsp)
-{
-   unsigned long flags;
-   struct rcu_node *rnp;
-
-   sync_exp_reset_tree_hotplug(rsp);
-   rcu_for_each_node_breadth_first(rsp, rnp) {
-   raw_spin_lock_irqsave_rcu_node(rnp, flags);
-   WARN_ON_ONCE(rnp->expmask);
-   rnp->expmask =