[PATCH tip/core/rcu 0/4] rcu inline, expedited, ->completed cleanups

[PATCH tip/core/rcu 0/4] rcu inline, expedited, ->completed cleanups

[Paul E. McKenney]

This patchset contains a few fixes from testing and review of treercu.
The review was spurred by the earlier bugs.

1.	Some compilers don't like forward references to inline functions.
	Make the compiler happy by removing "inline".

2.	Although synchronize_sched_expedited() was implemented with a
	fastpath to efficiently handle multiple concurrent calls,
	the counter increment that enabled this fastpath was omitted.
	Add it back in.  (And yes, this does make a big difference in
	performance and scalability for multiple concurrent calls to
	synchronize_sched_expedited(), not that such calls should happen
	all that often.  Still, no point in bringing the machine to its
	knees unnecessarily.)

3.	The field rsp->dynticks_completed is now used whether or not
	dynticks is enabled.  Rename it to rsp->completed_fqs to better
	document its new role.

4.	Much of the confusion leading to the bugs provoked by testing
	with long RCU grace periods was due to the method used to
	determine which grace period a given context-switch quiescent
	state is to be applied to.  This change provides a much more
	straightforward detection method for these quiescent states.
	The quiescent states induced by force_quiescent_state() still
	use the old method, and a separate patch for them is in the
	works.

[PATCH tip/core/rcu 1/4] rcu: remove inline from forward-referenced functions

[Paul E. McKenney]

Some variants of gcc are reputed to dislike forward references to
functions declared "inline".  Remove the "inline" keyword from such
functions.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcutree.h        |    2 +-
 kernel/rcutree_plugin.h |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/rcutree.h b/kernel/rcutree.h
index 5de1fe8..e2f07a2 100644
--- a/kernel/rcutree.h
+++ b/kernel/rcutree.h
@@ -301,7 +301,7 @@ DECLARE_PER_CPU(struct rcu_data, rcu_preempt_data);
 #else /* #ifdef RCU_TREE_NONCORE */
 
 /* Forward declarations for rcutree_plugin.h */
-static inline void rcu_bootup_announce(void);
+static void rcu_bootup_announce(void);
 long rcu_batches_completed(void);
 static void rcu_preempt_note_context_switch(int cpu);
 static int rcu_preempted_readers(struct rcu_node *rnp);
diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h
index ebd20ee..9744dfc 100644
--- a/kernel/rcutree_plugin.h
+++ b/kernel/rcutree_plugin.h
@@ -33,7 +33,7 @@ DEFINE_PER_CPU(struct rcu_data, rcu_preempt_data);
 /*
  * Tell them what RCU they are running.
  */
-static inline void rcu_bootup_announce(void)
+static void rcu_bootup_announce(void)
 {
 	printk(KERN_INFO
 	       "Experimental preemptable hierarchical RCU implementation.\n");
@@ -475,7 +475,7 @@ void exit_rcu(void)
 /*
  * Tell them what RCU they are running.
  */
-static inline void rcu_bootup_announce(void)
+static void rcu_bootup_announce(void)
 {
 	printk(KERN_INFO "Hierarchical RCU implementation.\n");
 }
-- 
1.5.2.5

[PATCH tip/core/rcu 2/4] rcu: enable synchronize_sched_expedited() fastpath

[Paul E. McKenney]

From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

This patch adds a counter increment to enable tasks to actually take
the synchronize_sched_expedited() function's fastpath.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/sched.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/kernel/sched.c b/kernel/sched.c
index 76c0e96..e69fee4 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -10865,6 +10865,7 @@ void synchronize_sched_expedited(void)
 		spin_unlock_irqrestore(&rq->lock, flags);
 	}
 	rcu_expedited_state = RCU_EXPEDITED_STATE_IDLE;
+	synchronize_sched_expedited_count++;
 	mutex_unlock(&rcu_sched_expedited_mutex);
 	put_online_cpus();
 	if (need_full_sync)
-- 
1.5.2.5

[PATCH tip/core/rcu 3/4] rcu: rename dynticks_completed to completed_fqs

[Paul E. McKenney]

From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

This field is used whether or not CONFIG_NO_HZ is set, so the old name
of ->dynticks_completed is quite misleading.  Change to ->completed_fqs,
given that it the value that force_quiescent_state() is trying to drive
the ->completed field away from.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcutree.c |    4 ++--
 kernel/rcutree.h |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index af9ea09..4026560 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -187,7 +187,7 @@ static struct rcu_node *rcu_get_root(struct rcu_state *rsp)
  */
 static void dyntick_record_completed(struct rcu_state *rsp, long comp)
 {
-	rsp->dynticks_completed = comp;
+	rsp->completed_fqs = comp;
 }
 
 #ifdef CONFIG_SMP
@@ -197,7 +197,7 @@ static void dyntick_record_completed(struct rcu_state *rsp, long comp)
  */
 static long dyntick_recall_completed(struct rcu_state *rsp)
 {
-	return rsp->dynticks_completed;
+	return rsp->completed_fqs;
 }
 
 /*
diff --git a/kernel/rcutree.h b/kernel/rcutree.h
index e2f07a2..f478503 100644
--- a/kernel/rcutree.h
+++ b/kernel/rcutree.h
@@ -264,6 +264,8 @@ struct rcu_state {
 	long orphan_qlen;			/* Number of orphaned cbs. */
 	spinlock_t fqslock;			/* Only one task forcing */
 						/*  quiescent states. */
+	long	completed_fqs;			/* Value of completed @ snap. */
+						/*  Protected by fqslock. */
 	unsigned long jiffies_force_qs;		/* Time at which to invoke */
 						/*  force_quiescent_state(). */
 	unsigned long n_force_qs;		/* Number of calls to */
@@ -278,8 +280,6 @@ struct rcu_state {
 	unsigned long jiffies_stall;		/* Time at which to check */
 						/*  for CPU stalls. */
 #endif /* #ifdef CONFIG_RCU_CPU_STALL_DETECTOR */
-	long dynticks_completed;		/* Value of completed @ snap. */
-						/*  Protected by fqslock. */
 };
 
 #ifdef RCU_TREE_NONCORE
-- 
1.5.2.5

[PATCH tip/core/rcu 4/4] rcu: simplify association of quiescent states with grace periods

[Paul E. McKenney]

From: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

The rdp->passed_quiesc_completed fields are used to properly associate
the recorded quiescent state with a grace period.  It is OK to wrongly
associate a given quiescent state with a preceding grace period, but it
is fatal to associate a given quiescent state with a grace period
that begins after the quiescent state occurred.  Grace periods are
numbered, and the following fields track them:

o	->gpnum is the number of the grace period currently in progress,
	or the number of the last grace period to complete if no grace
	period is currently in progress.

o	->completed is the number of the last grace period to have
	completed.

These two fields are equal if there is no grace period in progress,
otherwise ->gpnum is one greater than ->completed.  But the
rdp->passed_quiesc_completed field compared against ->completed,
and if equal, the quiescent state is presumed to count against the
current grace period.

The earlier code copied rdp->completed to rdp->passed_quiesc_completed,
which has been made to work, but is error-prone.  In contrast, copying
one less than rdp->gpnum is guaranteed safe, because rdp->gpnum is not
incremented until after the start of the corresponding grace period.
At the end of the grace period, when ->completed has incremented, then
any quiescent periods recorded previously will be discarded.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
---
 kernel/rcutree.c        |    4 ++--
 kernel/rcutree_plugin.h |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index 4026560..2f5d8c5 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -100,7 +100,7 @@ void rcu_sched_qs(int cpu)
 	struct rcu_data *rdp;
 
 	rdp = &per_cpu(rcu_sched_data, cpu);
-	rdp->passed_quiesc_completed = rdp->completed;
+	rdp->passed_quiesc_completed = rdp->gpnum - 1;
 	barrier();
 	rdp->passed_quiesc = 1;
 	rcu_preempt_note_context_switch(cpu);
@@ -111,7 +111,7 @@ void rcu_bh_qs(int cpu)
 	struct rcu_data *rdp;
 
 	rdp = &per_cpu(rcu_bh_data, cpu);
-	rdp->passed_quiesc_completed = rdp->completed;
+	rdp->passed_quiesc_completed = rdp->gpnum - 1;
 	barrier();
 	rdp->passed_quiesc = 1;
 }
diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h
index 9744dfc..b7dc0c2 100644
--- a/kernel/rcutree_plugin.h
+++ b/kernel/rcutree_plugin.h
@@ -67,7 +67,7 @@ EXPORT_SYMBOL_GPL(rcu_batches_completed);
 static void rcu_preempt_qs(int cpu)
 {
 	struct rcu_data *rdp = &per_cpu(rcu_preempt_data, cpu);
-	rdp->passed_quiesc_completed = rdp->completed;
+	rdp->passed_quiesc_completed = rdp->gpnum - 1;
 	barrier();
 	rdp->passed_quiesc = 1;
 }
-- 
1.5.2.5

[tip:core/rcu] rcu: Remove inline from forward-referenced functions

[tip-bot for Paul E. McKenney]

Commit-ID:  dbe01350fa8ce0c11948ab7d6be71a4d901be151
Gitweb:     http://git.kernel.org/tip/dbe01350fa8ce0c11948ab7d6be71a4d901be151
Author:     Paul E. McKenney <paulmck@linux.vnet.ibm.com>
AuthorDate: Tue, 10 Nov 2009 13:37:19 -0800
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Tue, 10 Nov 2009 22:48:49 +0100

rcu: Remove inline from forward-referenced functions

Some variants of gcc are reputed to dislike forward references
to functions declared "inline".  Remove the "inline" keyword
from such functions.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: laijs@cn.fujitsu.com
Cc: dipankar@in.ibm.com
Cc: mathieu.desnoyers@polymtl.ca
Cc: josh@joshtriplett.org
Cc: dvhltc@us.ibm.com
Cc: niv@us.ibm.com
Cc: peterz@infradead.org
Cc: rostedt@goodmis.org
Cc: Valdis.Kletnieks@vt.edu
Cc: dhowells@redhat.com
LKML-Reference: <12578890422402-git-send-email->
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/rcutree.h        |    2 +-
 kernel/rcutree_plugin.h |    4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/rcutree.h b/kernel/rcutree.h
index c1891c3..ddb79ec 100644
--- a/kernel/rcutree.h
+++ b/kernel/rcutree.h
@@ -301,7 +301,7 @@ DECLARE_PER_CPU(struct rcu_data, rcu_preempt_data);
 #else /* #ifdef RCU_TREE_NONCORE */
 
 /* Forward declarations for rcutree_plugin.h */
-static inline void rcu_bootup_announce(void);
+static void rcu_bootup_announce(void);
 long rcu_batches_completed(void);
 static void rcu_preempt_note_context_switch(int cpu);
 static int rcu_preempted_readers(struct rcu_node *rnp);
diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h
index ef2a58c..c03edf7 100644
--- a/kernel/rcutree_plugin.h
+++ b/kernel/rcutree_plugin.h
@@ -33,7 +33,7 @@ DEFINE_PER_CPU(struct rcu_data, rcu_preempt_data);
 /*
  * Tell them what RCU they are running.
  */
-static inline void rcu_bootup_announce(void)
+static void rcu_bootup_announce(void)
 {
 	printk(KERN_INFO
 	       "Experimental preemptable hierarchical RCU implementation.\n");
@@ -481,7 +481,7 @@ void exit_rcu(void)
 /*
  * Tell them what RCU they are running.
  */
-static inline void rcu_bootup_announce(void)
+static void rcu_bootup_announce(void)
 {
 	printk(KERN_INFO "Hierarchical RCU implementation.\n");
 }

[tip:core/rcu] rcu: Enable synchronize_sched_expedited() fastpath

[tip-bot for Paul E. McKenney]

Commit-ID:  956539b75921f561c0956c22d37320780e8b4ba1
Gitweb:     http://git.kernel.org/tip/956539b75921f561c0956c22d37320780e8b4ba1
Author:     Paul E. McKenney <paulmck@linux.vnet.ibm.com>
AuthorDate: Tue, 10 Nov 2009 13:37:20 -0800
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Tue, 10 Nov 2009 22:48:49 +0100

rcu: Enable synchronize_sched_expedited() fastpath

This patch adds a counter increment to enable tasks to actually
take the synchronize_sched_expedited() function's fastpath.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: laijs@cn.fujitsu.com
Cc: dipankar@in.ibm.com
Cc: mathieu.desnoyers@polymtl.ca
Cc: josh@joshtriplett.org
Cc: dvhltc@us.ibm.com
Cc: niv@us.ibm.com
Cc: peterz@infradead.org
Cc: rostedt@goodmis.org
Cc: Valdis.Kletnieks@vt.edu
Cc: dhowells@redhat.com
LKML-Reference: <1257889042435-git-send-email->
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/sched.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/kernel/sched.c b/kernel/sched.c
index 76c0e96..e69fee4 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -10865,6 +10865,7 @@ void synchronize_sched_expedited(void)
 		spin_unlock_irqrestore(&rq->lock, flags);
 	}
 	rcu_expedited_state = RCU_EXPEDITED_STATE_IDLE;
+	synchronize_sched_expedited_count++;
 	mutex_unlock(&rcu_sched_expedited_mutex);
 	put_online_cpus();
 	if (need_full_sync)

[tip:core/rcu] rcu: Rename dynticks_completed to completed_fqs

[tip-bot for Paul E. McKenney]

Commit-ID:  4bcfe055030d9e953945def3864f7e6997b27782
Gitweb:     http://git.kernel.org/tip/4bcfe055030d9e953945def3864f7e6997b27782
Author:     Paul E. McKenney <paulmck@linux.vnet.ibm.com>
AuthorDate: Tue, 10 Nov 2009 13:37:21 -0800
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Tue, 10 Nov 2009 22:48:50 +0100

rcu: Rename dynticks_completed to completed_fqs

This field is used whether or not CONFIG_NO_HZ is set, so the
old name of ->dynticks_completed is quite misleading.

Change to ->completed_fqs, given that it the value that
force_quiescent_state() is trying to drive the ->completed field
away from.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: laijs@cn.fujitsu.com
Cc: dipankar@in.ibm.com
Cc: mathieu.desnoyers@polymtl.ca
Cc: josh@joshtriplett.org
Cc: dvhltc@us.ibm.com
Cc: niv@us.ibm.com
Cc: peterz@infradead.org
Cc: rostedt@goodmis.org
Cc: Valdis.Kletnieks@vt.edu
Cc: dhowells@redhat.com
LKML-Reference: <12578890423298-git-send-email->
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/rcutree.c |    4 ++--
 kernel/rcutree.h |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index ec007dd..26fc780 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -187,7 +187,7 @@ static struct rcu_node *rcu_get_root(struct rcu_state *rsp)
  */
 static void dyntick_record_completed(struct rcu_state *rsp, long comp)
 {
-	rsp->dynticks_completed = comp;
+	rsp->completed_fqs = comp;
 }
 
 #ifdef CONFIG_SMP
@@ -197,7 +197,7 @@ static void dyntick_record_completed(struct rcu_state *rsp, long comp)
  */
 static long dyntick_recall_completed(struct rcu_state *rsp)
 {
-	return rsp->dynticks_completed;
+	return rsp->completed_fqs;
 }
 
 /*
diff --git a/kernel/rcutree.h b/kernel/rcutree.h
index ddb79ec..17a28a0 100644
--- a/kernel/rcutree.h
+++ b/kernel/rcutree.h
@@ -264,6 +264,8 @@ struct rcu_state {
 	long orphan_qlen;			/* Number of orphaned cbs. */
 	spinlock_t fqslock;			/* Only one task forcing */
 						/*  quiescent states. */
+	long	completed_fqs;			/* Value of completed @ snap. */
+						/*  Protected by fqslock. */
 	unsigned long jiffies_force_qs;		/* Time at which to invoke */
 						/*  force_quiescent_state(). */
 	unsigned long n_force_qs;		/* Number of calls to */
@@ -278,8 +280,6 @@ struct rcu_state {
 	unsigned long jiffies_stall;		/* Time at which to check */
 						/*  for CPU stalls. */
 #endif /* #ifdef CONFIG_RCU_CPU_STALL_DETECTOR */
-	long dynticks_completed;		/* Value of completed @ snap. */
-						/*  Protected by fqslock. */
 };
 
 #ifdef RCU_TREE_NONCORE

[tip:core/rcu] rcu: Simplify association of quiescent states with grace periods

[tip-bot for Paul E. McKenney]

Commit-ID:  c64ac3ce06558e534aec62b1fadeb0a3f111dac1
Gitweb:     http://git.kernel.org/tip/c64ac3ce06558e534aec62b1fadeb0a3f111dac1
Author:     Paul E. McKenney <paulmck@linux.vnet.ibm.com>
AuthorDate: Tue, 10 Nov 2009 13:37:22 -0800
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Tue, 10 Nov 2009 22:48:50 +0100

rcu: Simplify association of quiescent states with grace periods

The rdp->passed_quiesc_completed fields are used to properly
associate the recorded quiescent state with a grace period.  It
is OK to wrongly associate a given quiescent state with a
preceding grace period, but it is fatal to associate a given
quiescent state with a grace period that begins after the
quiescent state occurred.  Grace periods are numbered, and the
following fields track them:

o	->gpnum is the number of the grace period currently in
	progress, or the number of the last grace period to
	complete if no grace period is currently in progress.

o	->completed is the number of the last grace period to
	have completed.

These two fields are equal if there is no grace period in
progress, otherwise ->gpnum is one greater than ->completed.
But the rdp->passed_quiesc_completed field compared against
->completed, and if equal, the quiescent state is presumed to
count against the current grace period.

The earlier code copied rdp->completed to
rdp->passed_quiesc_completed, which has been made to work, but
is error-prone.  In contrast, copying one less than rdp->gpnum
is guaranteed safe, because rdp->gpnum is not incremented until
after the start of the corresponding grace period. At the end of
the grace period, when ->completed has incremented, then any
quiescent periods recorded previously will be discarded.

Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: laijs@cn.fujitsu.com
Cc: dipankar@in.ibm.com
Cc: mathieu.desnoyers@polymtl.ca
Cc: josh@joshtriplett.org
Cc: dvhltc@us.ibm.com
Cc: niv@us.ibm.com
Cc: peterz@infradead.org
Cc: rostedt@goodmis.org
Cc: Valdis.Kletnieks@vt.edu
Cc: dhowells@redhat.com
LKML-Reference: <12578890421011-git-send-email->
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/rcutree.c        |    4 ++--
 kernel/rcutree_plugin.h |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index 26fc780..d802419 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -100,7 +100,7 @@ void rcu_sched_qs(int cpu)
 	struct rcu_data *rdp;
 
 	rdp = &per_cpu(rcu_sched_data, cpu);
-	rdp->passed_quiesc_completed = rdp->completed;
+	rdp->passed_quiesc_completed = rdp->gpnum - 1;
 	barrier();
 	rdp->passed_quiesc = 1;
 	rcu_preempt_note_context_switch(cpu);
@@ -111,7 +111,7 @@ void rcu_bh_qs(int cpu)
 	struct rcu_data *rdp;
 
 	rdp = &per_cpu(rcu_bh_data, cpu);
-	rdp->passed_quiesc_completed = rdp->completed;
+	rdp->passed_quiesc_completed = rdp->gpnum - 1;
 	barrier();
 	rdp->passed_quiesc = 1;
 }
diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h
index c03edf7..52075da 100644
--- a/kernel/rcutree_plugin.h
+++ b/kernel/rcutree_plugin.h
@@ -67,7 +67,7 @@ EXPORT_SYMBOL_GPL(rcu_batches_completed);
 static void rcu_preempt_qs(int cpu)
 {
 	struct rcu_data *rdp = &per_cpu(rcu_preempt_data, cpu);
-	rdp->passed_quiesc_completed = rdp->completed;
+	rdp->passed_quiesc_completed = rdp->gpnum - 1;
 	barrier();
 	rdp->passed_quiesc = 1;
 }

Re: [tip:core/rcu] rcu: Remove inline from forward-referenced functions

[Joe Perches]

On Tue, 2009-11-10 at 22:27 +0000, tip-bot for Paul E. McKenney wrote:
> rcu: Remove inline from forward-referenced functions
> diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h
> index ef2a58c..c03edf7 100644
> --- a/kernel/rcutree_plugin.h
> +++ b/kernel/rcutree_plugin.h
> @@ -33,7 +33,7 @@ DEFINE_PER_CPU(struct rcu_data, rcu_preempt_data);
>  /*
>   * Tell them what RCU they are running.
>   */
> -static inline void rcu_bootup_announce(void)
> +static void rcu_bootup_announce(void)
>  {
>  	printk(KERN_INFO
>  	       "Experimental preemptable hierarchical RCU implementation.\n");
> @@ -481,7 +481,7 @@ void exit_rcu(void)
>  /*
>   * Tell them what RCU they are running.
>   */
> -static inline void rcu_bootup_announce(void)
> +static void rcu_bootup_announce(void)
>  {
>  	printk(KERN_INFO "Hierarchical RCU implementation.\n");
>  }

non-inline functions in .h files probably aren't a good idea.

Re: [tip:core/rcu] rcu: Remove inline from forward-referenced functions

[Paul E. McKenney]

On Tue, Nov 10, 2009 at 02:38:30PM -0800, Joe Perches wrote:
> On Tue, 2009-11-10 at 22:27 +0000, tip-bot for Paul E. McKenney wrote:
> > rcu: Remove inline from forward-referenced functions
> > diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h
> > index ef2a58c..c03edf7 100644
> > --- a/kernel/rcutree_plugin.h
> > +++ b/kernel/rcutree_plugin.h
> > @@ -33,7 +33,7 @@ DEFINE_PER_CPU(struct rcu_data, rcu_preempt_data);
> >  /*
> >   * Tell them what RCU they are running.
> >   */
> > -static inline void rcu_bootup_announce(void)
> > +static void rcu_bootup_announce(void)
> >  {
> >  	printk(KERN_INFO
> >  	       "Experimental preemptable hierarchical RCU implementation.\n");
> > @@ -481,7 +481,7 @@ void exit_rcu(void)
> >  /*
> >   * Tell them what RCU they are running.
> >   */
> > -static inline void rcu_bootup_announce(void)
> > +static void rcu_bootup_announce(void)
> >  {
> >  	printk(KERN_INFO "Hierarchical RCU implementation.\n");
> >  }
> 
> non-inline functions in .h files probably aren't a good idea.

;-)

Here are my options:

1.	Have a huge #ifdef in kernel/rcutree.c.  Not going there.

2.	#include kernel/rcutree_plugin.h near the beginning of
	kernel/rcutree.c rather than near the end.  I originally
	had it set up this way, but this results in a large number
	of forward references from kernel/rcutree_plugin.h, which
	turned out to be a real mess.  The current setup is much
	nicer, as the forward references serve to document the
	plug-in functions that are defined in kernel/rcutree_plugin.h.

3.	Leave in the "inline" declarations on the function definitions.
	Although all the compilers that -I- use are quite happy with
	this, some people's compilers complain.  So this is not good,
	either.

Please note that kernel/rcutree_plugin.h is internal to RCU -- only
kernel/rcutree.c includes it, so there is no possibility of conflicting
definitions.

Any options that I am missing?

							Thanx, Paul

Re: [PATCH tip/core/rcu 1/4] rcu: remove inline from forward-referenced functions

[Josh Triplett]

On Tue, Nov 10, 2009 at 01:37:19PM -0800, Paul E. McKenney wrote:
> Some variants of gcc are reputed to dislike forward references to
> functions declared "inline".  Remove the "inline" keyword from such
> functions.
> 
> Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> ---
>  kernel/rcutree.h        |    2 +-
>  kernel/rcutree_plugin.h |    4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)

Seems very reasonable.  For functions declared static and called once, GCC doesn't
really need any help figuring out that it should inline them.

Acked-by: Josh Triplett <josh@joshtriplett.org>

Re: [tip:core/rcu] rcu: Remove inline from forward-referenced functions

[Joe Perches]

On Tue, 2009-11-10 at 14:59 -0800, Paul E. McKenney wrote:
> > non-inline functions in .h files probably aren't a good idea.
> ;-)
> Here are my options:
[]
> Please note that kernel/rcutree_plugin.h is internal to RCU -- only
> kernel/rcutree.c includes it, so there is no possibility of conflicting
> definitions.
> 
> Any options that I am missing?

Maybe something like:

#ifdef whatever
#define RCU_ANNOUNCE "Hierarchical RCU implementation.\n"
...
#else
#define RCU_ANNOUNCE "Experimental preemptable hierarchical RCU implementation.\n"
...
#endif

Use pr_info(RCU_ANNOUNCE) instead of rcu_bootup_announce();

?

Re: [tip:core/rcu] rcu: Remove inline from forward-referenced functions

[Paul E. McKenney]

On Tue, Nov 10, 2009 at 03:06:20PM -0800, Joe Perches wrote:
> On Tue, 2009-11-10 at 14:59 -0800, Paul E. McKenney wrote:
> > > non-inline functions in .h files probably aren't a good idea.
> > ;-)
> > Here are my options:
> []
> > Please note that kernel/rcutree_plugin.h is internal to RCU -- only
> > kernel/rcutree.c includes it, so there is no possibility of conflicting
> > definitions.
> > 
> > Any options that I am missing?
> 
> Maybe something like:
> 
> #ifdef whatever
> #define RCU_ANNOUNCE "Hierarchical RCU implementation.\n"
> ...
> #else
> #define RCU_ANNOUNCE "Experimental preemptable hierarchical RCU implementation.\n"
> ...
> #endif
> 
> Use pr_info(RCU_ANNOUNCE) instead of rcu_bootup_announce();
> 
> ?

This would still be a forward reference, right?  Unless I am missing
something, changing from a static inline to a cpp macro doesn't change
anything.

							Thanx, Paul

Re: [tip:core/rcu] rcu: Remove inline from forward-referenced functions

[Joe Perches]

On Tue, 2009-11-10 at 15:41 -0800, Paul E. McKenney wrote:
> On Tue, Nov 10, 2009 at 03:06:20PM -0800, Joe Perches wrote:
> > On Tue, 2009-11-10 at 14:59 -0800, Paul E. McKenney wrote:
> > > > non-inline functions in .h files probably aren't a good idea.
> > > ;-)
> > > Here are my options:
> > []
> > > Please note that kernel/rcutree_plugin.h is internal to RCU -- only
> > > kernel/rcutree.c includes it, so there is no possibility of conflicting
> > > definitions.
> > > Any options that I am missing?
> > Maybe something like:
> > #ifdef whatever
> > #define RCU_ANNOUNCE "Hierarchical RCU implementation.\n"
> > ...
> > #else
> > #define RCU_ANNOUNCE "Experimental preemptable hierarchical RCU implementation.\n"
> > ...
> > #endif
> > Use pr_info(RCU_ANNOUNCE) instead of rcu_bootup_announce();
> > ?
> This would still be a forward reference, right?

No, what I suggest would delete the forward reference and
the  rcu_bootup_announce function altogether.

The single caller of rcu_bootup_announce in __rcu_init
would become printk(KERN_INFO RCU_ANNOUNCE);

I wrote the original quoted content above without looking
at the file, just the submitted patch.  After looking the
the rcutree files, another option is to do what Ingo did
with sched.c and finesse the #include ".h".

$ grep "include.*\.c" sched.c
#include "sched_idletask.c"
#include "sched_fair.c"
#include "sched_rt.c"
# include "sched_debug.c"

So maybe rename rcutree_plugin.h to rcutree_plugin.c and
#include "rcutree_plugin.c" in rcutree.c instead.

cheers, Joe

Re: [tip:core/rcu] rcu: Remove inline from forward-referenced functions

[Paul E. McKenney]

On Tue, Nov 10, 2009 at 05:03:41PM -0800, Joe Perches wrote:
> On Tue, 2009-11-10 at 15:41 -0800, Paul E. McKenney wrote:
> > On Tue, Nov 10, 2009 at 03:06:20PM -0800, Joe Perches wrote:
> > > On Tue, 2009-11-10 at 14:59 -0800, Paul E. McKenney wrote:
> > > > > non-inline functions in .h files probably aren't a good idea.
> > > > ;-)
> > > > Here are my options:
> > > []
> > > > Please note that kernel/rcutree_plugin.h is internal to RCU -- only
> > > > kernel/rcutree.c includes it, so there is no possibility of conflicting
> > > > definitions.
> > > > Any options that I am missing?
> > > Maybe something like:
> > > #ifdef whatever
> > > #define RCU_ANNOUNCE "Hierarchical RCU implementation.\n"
> > > ...
> > > #else
> > > #define RCU_ANNOUNCE "Experimental preemptable hierarchical RCU implementation.\n"
> > > ...
> > > #endif
> > > Use pr_info(RCU_ANNOUNCE) instead of rcu_bootup_announce();
> > > ?
> > This would still be a forward reference, right?
> 
> No, what I suggest would delete the forward reference and
> the  rcu_bootup_announce function altogether.

I still either have a plugin definition in the non-plugin file
kernel/rcutree.c or a forward reference to the macro definition.

> The single caller of rcu_bootup_announce in __rcu_init
> would become printk(KERN_INFO RCU_ANNOUNCE);
> 
> I wrote the original quoted content above without looking
> at the file, just the submitted patch.  After looking the
> the rcutree files, another option is to do what Ingo did
> with sched.c and finesse the #include ".h".
> 
> $ grep "include.*\.c" sched.c
> #include "sched_idletask.c"
> #include "sched_fair.c"
> #include "sched_rt.c"
> # include "sched_debug.c"
> 
> So maybe rename rcutree_plugin.h to rcutree_plugin.c and
> #include "rcutree_plugin.c" in rcutree.c instead.

Hmmm...

							Thanx, Paul

Re: [tip:core/rcu] rcu: Remove inline from forward-referenced functions

[Joe Perches]

On Tue, 2009-11-10 at 17:42 -0800, Paul E. McKenney wrote:
> On Tue, Nov 10, 2009 at 05:03:41PM -0800, Joe Perches wrote:
> > So maybe rename rcutree_plugin.h to rcutree_plugin.c and
> > #include "rcutree_plugin.c" in rcutree.c instead.
> Hmmm...

Perhaps something like this:

cheers, Joe

rcu: rename rcutree_plugin.h to .c and use

diff --git a/kernel/rcutree.c b/kernel/rcutree.c
index d802419..737dc72 100644
--- a/kernel/rcutree.c
+++ b/kernel/rcutree.c
@@ -1783,6 +1783,8 @@ do { \
 	} \
 } while (0)
 
+#include "rcutree_plugin.c"
+
 void __init __rcu_init(void)
 {
 	rcu_bootup_announce();
@@ -1795,4 +1797,3 @@ void __init __rcu_init(void)
 	open_softirq(RCU_SOFTIRQ, rcu_process_callbacks);
 }
 
-#include "rcutree_plugin.h"
diff --git a/kernel/rcutree.h b/kernel/rcutree.h
index 17a28a0..ed11d16 100644
--- a/kernel/rcutree.h
+++ b/kernel/rcutree.h
@@ -300,8 +300,7 @@ DECLARE_PER_CPU(struct rcu_data, rcu_preempt_data);
 
 #else /* #ifdef RCU_TREE_NONCORE */
 
-/* Forward declarations for rcutree_plugin.h */
-static void rcu_bootup_announce(void);
+/* Forward declarations for rcutree_plugin.c */
 long rcu_batches_completed(void);
 static void rcu_preempt_note_context_switch(int cpu);
 static int rcu_preempted_readers(struct rcu_node *rnp);
diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.c
similarity index 99%
rename from kernel/rcutree_plugin.h
rename to kernel/rcutree_plugin.c
index 52075da..5ca2d26 100644
--- a/kernel/rcutree_plugin.h
+++ b/kernel/rcutree_plugin.c
@@ -33,7 +33,7 @@ DEFINE_PER_CPU(struct rcu_data, rcu_preempt_data);
 /*
  * Tell them what RCU they are running.
  */
-static void rcu_bootup_announce(void)
+static void __init rcu_bootup_announce(void)
 {
 	printk(KERN_INFO
 	       "Experimental preemptable hierarchical RCU implementation.\n");
@@ -481,7 +481,7 @@ void exit_rcu(void)
 /*
  * Tell them what RCU they are running.
  */
-static void rcu_bootup_announce(void)
+static void __init rcu_bootup_announce(void)
 {
 	printk(KERN_INFO "Hierarchical RCU implementation.\n");
 }

Re: [tip:core/rcu] rcu: Remove inline from forward-referenced functions

[Paul E. McKenney]

On Tue, Nov 10, 2009 at 07:28:28PM -0800, Joe Perches wrote:
> On Tue, 2009-11-10 at 17:42 -0800, Paul E. McKenney wrote:
> > On Tue, Nov 10, 2009 at 05:03:41PM -0800, Joe Perches wrote:
> > > So maybe rename rcutree_plugin.h to rcutree_plugin.c and
> > > #include "rcutree_plugin.c" in rcutree.c instead.
> > Hmmm...
> 
> Perhaps something like this:

While I do very much appreciate your time and attention to this...

My problem with this sort of thing is that when I tried it, it proved
fragile.  Small changes required lots of rework of forward declarations.
Putting it at the end makes it work very nicely -- the list of forward
declarations doubles as documentation for the plugins, and the contents
of kernel/rcutree_plugin.h (or .c or whatever, either way I end up
violation about the same number of coding guidelines) is independent of
rearrangements of kernel/rcutree.c.

The reason that I would really like to keep rcu_bootup_announce() as
a function is that it makes it trivial to collect RCU-flavor-dependent
boot-time information, if needed for some debugging effort.  If I pull
the string out, this sort of thing becomes much more painful.

							Thanx, Paul

> cheers, Joe
> 
> rcu: rename rcutree_plugin.h to .c and use
> 
> diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> index d802419..737dc72 100644
> --- a/kernel/rcutree.c
> +++ b/kernel/rcutree.c
> @@ -1783,6 +1783,8 @@ do { \
>  	} \
>  } while (0)
> 
> +#include "rcutree_plugin.c"
> +
>  void __init __rcu_init(void)
>  {
>  	rcu_bootup_announce();
> @@ -1795,4 +1797,3 @@ void __init __rcu_init(void)
>  	open_softirq(RCU_SOFTIRQ, rcu_process_callbacks);
>  }
> 
> -#include "rcutree_plugin.h"
> diff --git a/kernel/rcutree.h b/kernel/rcutree.h
> index 17a28a0..ed11d16 100644
> --- a/kernel/rcutree.h
> +++ b/kernel/rcutree.h
> @@ -300,8 +300,7 @@ DECLARE_PER_CPU(struct rcu_data, rcu_preempt_data);
> 
>  #else /* #ifdef RCU_TREE_NONCORE */
> 
> -/* Forward declarations for rcutree_plugin.h */
> -static void rcu_bootup_announce(void);
> +/* Forward declarations for rcutree_plugin.c */
>  long rcu_batches_completed(void);
>  static void rcu_preempt_note_context_switch(int cpu);
>  static int rcu_preempted_readers(struct rcu_node *rnp);
> diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.c
> similarity index 99%
> rename from kernel/rcutree_plugin.h
> rename to kernel/rcutree_plugin.c
> index 52075da..5ca2d26 100644
> --- a/kernel/rcutree_plugin.h
> +++ b/kernel/rcutree_plugin.c
> @@ -33,7 +33,7 @@ DEFINE_PER_CPU(struct rcu_data, rcu_preempt_data);
>  /*
>   * Tell them what RCU they are running.
>   */
> -static void rcu_bootup_announce(void)
> +static void __init rcu_bootup_announce(void)
>  {
>  	printk(KERN_INFO
>  	       "Experimental preemptable hierarchical RCU implementation.\n");
> @@ -481,7 +481,7 @@ void exit_rcu(void)
>  /*
>   * Tell them what RCU they are running.
>   */
> -static void rcu_bootup_announce(void)
> +static void __init rcu_bootup_announce(void)
>  {
>  	printk(KERN_INFO "Hierarchical RCU implementation.\n");
>  }
> 
> 

Re: [tip:core/rcu] rcu: Remove inline from forward-referenced functions

[Paul E. McKenney]

On Tue, Nov 10, 2009 at 08:50:13PM -0800, Paul E. McKenney wrote:
> On Tue, Nov 10, 2009 at 07:28:28PM -0800, Joe Perches wrote:
> > On Tue, 2009-11-10 at 17:42 -0800, Paul E. McKenney wrote:
> > > On Tue, Nov 10, 2009 at 05:03:41PM -0800, Joe Perches wrote:
> > > > So maybe rename rcutree_plugin.h to rcutree_plugin.c and
> > > > #include "rcutree_plugin.c" in rcutree.c instead.
> > > Hmmm...
> > 
> > Perhaps something like this:
> 
> While I do very much appreciate your time and attention to this...
> 
> My problem with this sort of thing is that when I tried it, it proved
> fragile.  Small changes required lots of rework of forward declarations.
> Putting it at the end makes it work very nicely -- the list of forward
> declarations doubles as documentation for the plugins, and the contents
> of kernel/rcutree_plugin.h (or .c or whatever, either way I end up
> violation about the same number of coding guidelines) is independent of
> rearrangements of kernel/rcutree.c.
> 
> The reason that I would really like to keep rcu_bootup_announce() as
> a function is that it makes it trivial to collect RCU-flavor-dependent
> boot-time information, if needed for some debugging effort.  If I pull
> the string out, this sort of thing becomes much more painful.

And, as noted in our offline conversation, you are absolutely right
that I need to add __init to both definitions of rcu_bootup_announce(),
which I will do, with your Suggested-by.

Fair enough?

							Thanx, Paul

> > cheers, Joe
> > 
> > rcu: rename rcutree_plugin.h to .c and use
> > 
> > diff --git a/kernel/rcutree.c b/kernel/rcutree.c
> > index d802419..737dc72 100644
> > --- a/kernel/rcutree.c
> > +++ b/kernel/rcutree.c
> > @@ -1783,6 +1783,8 @@ do { \
> >  	} \
> >  } while (0)
> > 
> > +#include "rcutree_plugin.c"
> > +
> >  void __init __rcu_init(void)
> >  {
> >  	rcu_bootup_announce();
> > @@ -1795,4 +1797,3 @@ void __init __rcu_init(void)
> >  	open_softirq(RCU_SOFTIRQ, rcu_process_callbacks);
> >  }
> > 
> > -#include "rcutree_plugin.h"
> > diff --git a/kernel/rcutree.h b/kernel/rcutree.h
> > index 17a28a0..ed11d16 100644
> > --- a/kernel/rcutree.h
> > +++ b/kernel/rcutree.h
> > @@ -300,8 +300,7 @@ DECLARE_PER_CPU(struct rcu_data, rcu_preempt_data);
> > 
> >  #else /* #ifdef RCU_TREE_NONCORE */
> > 
> > -/* Forward declarations for rcutree_plugin.h */
> > -static void rcu_bootup_announce(void);
> > +/* Forward declarations for rcutree_plugin.c */
> >  long rcu_batches_completed(void);
> >  static void rcu_preempt_note_context_switch(int cpu);
> >  static int rcu_preempted_readers(struct rcu_node *rnp);
> > diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.c
> > similarity index 99%
> > rename from kernel/rcutree_plugin.h
> > rename to kernel/rcutree_plugin.c
> > index 52075da..5ca2d26 100644
> > --- a/kernel/rcutree_plugin.h
> > +++ b/kernel/rcutree_plugin.c
> > @@ -33,7 +33,7 @@ DEFINE_PER_CPU(struct rcu_data, rcu_preempt_data);
> >  /*
> >   * Tell them what RCU they are running.
> >   */
> > -static void rcu_bootup_announce(void)
> > +static void __init rcu_bootup_announce(void)
> >  {
> >  	printk(KERN_INFO
> >  	       "Experimental preemptable hierarchical RCU implementation.\n");
> > @@ -481,7 +481,7 @@ void exit_rcu(void)
> >  /*
> >   * Tell them what RCU they are running.
> >   */
> > -static void rcu_bootup_announce(void)
> > +static void __init rcu_bootup_announce(void)
> >  {
> >  	printk(KERN_INFO "Hierarchical RCU implementation.\n");
> >  }
> > 
> > 

Re: [tip:core/rcu] rcu: Remove inline from forward-referenced functions

[Ingo Molnar]

* Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:

> On Tue, Nov 10, 2009 at 08:50:13PM -0800, Paul E. McKenney wrote:
> > On Tue, Nov 10, 2009 at 07:28:28PM -0800, Joe Perches wrote:
> > > On Tue, 2009-11-10 at 17:42 -0800, Paul E. McKenney wrote:
> > > > On Tue, Nov 10, 2009 at 05:03:41PM -0800, Joe Perches wrote:
> > > > > So maybe rename rcutree_plugin.h to rcutree_plugin.c and
> > > > > #include "rcutree_plugin.c" in rcutree.c instead.
> > > > Hmmm...
> > > 
> > > Perhaps something like this:
> > 
> > While I do very much appreciate your time and attention to this...
> > 
> > My problem with this sort of thing is that when I tried it, it proved
> > fragile.  Small changes required lots of rework of forward declarations.
> > Putting it at the end makes it work very nicely -- the list of forward
> > declarations doubles as documentation for the plugins, and the contents
> > of kernel/rcutree_plugin.h (or .c or whatever, either way I end up
> > violation about the same number of coding guidelines) is independent of
> > rearrangements of kernel/rcutree.c.
> > 
> > The reason that I would really like to keep rcu_bootup_announce() as
> > a function is that it makes it trivial to collect RCU-flavor-dependent
> > boot-time information, if needed for some debugging effort.  If I pull
> > the string out, this sort of thing becomes much more painful.
> 
> And, as noted in our offline conversation, you are absolutely right
> that I need to add __init to both definitions of rcu_bootup_announce(),
> which I will do, with your Suggested-by.
> 
> Fair enough?

Yep, the __init markers are fair enough - but otherwise i wouldnt overdo 
this - a casual glance at rcutree_plugin.h shows that it's special, 
contains an implementation that is included once into kernel/rcutree.c. 
No need for header guards or a rename.

	Ingo

Re: [tip:core/rcu] rcu: Remove inline from forward-referenced functions

[Paul E. McKenney]

On Wed, Nov 11, 2009 at 08:02:08AM +0100, Ingo Molnar wrote:
> 
> * Paul E. McKenney <paulmck@linux.vnet.ibm.com> wrote:
> 
> > On Tue, Nov 10, 2009 at 08:50:13PM -0800, Paul E. McKenney wrote:
> > > On Tue, Nov 10, 2009 at 07:28:28PM -0800, Joe Perches wrote:
> > > > On Tue, 2009-11-10 at 17:42 -0800, Paul E. McKenney wrote:
> > > > > On Tue, Nov 10, 2009 at 05:03:41PM -0800, Joe Perches wrote:
> > > > > > So maybe rename rcutree_plugin.h to rcutree_plugin.c and
> > > > > > #include "rcutree_plugin.c" in rcutree.c instead.
> > > > > Hmmm...
> > > > 
> > > > Perhaps something like this:
> > > 
> > > While I do very much appreciate your time and attention to this...
> > > 
> > > My problem with this sort of thing is that when I tried it, it proved
> > > fragile.  Small changes required lots of rework of forward declarations.
> > > Putting it at the end makes it work very nicely -- the list of forward
> > > declarations doubles as documentation for the plugins, and the contents
> > > of kernel/rcutree_plugin.h (or .c or whatever, either way I end up
> > > violation about the same number of coding guidelines) is independent of
> > > rearrangements of kernel/rcutree.c.
> > > 
> > > The reason that I would really like to keep rcu_bootup_announce() as
> > > a function is that it makes it trivial to collect RCU-flavor-dependent
> > > boot-time information, if needed for some debugging effort.  If I pull
> > > the string out, this sort of thing becomes much more painful.
> > 
> > And, as noted in our offline conversation, you are absolutely right
> > that I need to add __init to both definitions of rcu_bootup_announce(),
> > which I will do, with your Suggested-by.
> > 
> > Fair enough?
> 
> Yep, the __init markers are fair enough - but otherwise i wouldnt overdo 
> this - a casual glance at rcutree_plugin.h shows that it's special, 
> contains an implementation that is included once into kernel/rcutree.c. 
> No need for header guards or a rename.

Sounds good, patch sent separately.  ;-)

							Thanx, Paul