All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] rcutree: Fix v3.4-rc2-rt2 build break
@ 2012-04-11 14:42 ` John Kacur
  0 siblings, 0 replies; 7+ messages in thread
From: John Kacur @ 2012-04-11 14:42 UTC (permalink / raw)
  To: Thomas Gleixner, Paul McKenney; +Cc: lkml, rt-users, John Kacur

Fix build break of the following types.

linux-rt/kernel/rcutree_plugin.h: In function ‘print_cpu_stall_fast_no_hz’:
linux-rt/kernel/rcutree_plugin.h:2195: error: ‘rcu_idle_gp_timer’ undeclared (first use in this function)
linux-rt/kernel/rcutree_plugin.h:2195: error: (Each undeclared identifier is reported only once
linux-rt/kernel/rcutree_plugin.h:2195: error: for each function it appears in.)

The build break only occurs with the PREEMPT_RT_FULL patch applied, however
the patch is meant to go upstream and be applied to v3.4-rc2 as well because
it makes the code more legible there, and will reduce the number of places
where #ifdef PREEMPT_RT_FULL is required should that go upstream someday.

Signed-off-by: John Kacur <jkacur@redhat.com>
---
 kernel/rcutree_plugin.h |   19 +++++++++----------
 1 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h
index 14acafc..1db9471 100644
--- a/kernel/rcutree_plugin.h
+++ b/kernel/rcutree_plugin.h
@@ -1938,6 +1938,12 @@ static void rcu_prepare_for_idle(int cpu)
 {
 }
 
+#ifdef CONFIG_RCU_CPU_STALL_INFO
+static void print_cpu_stall_fast_no_hz(char *cp, int cpu)
+{
+}
+#endif /* #ifdef CONFIG_RCU_CPU_STALL_INFO */
+
 #else /* #if !defined(CONFIG_RCU_FAST_NO_HZ) */
 
 /*
@@ -2184,12 +2190,8 @@ static void rcu_prepare_for_idle(int cpu)
 		trace_rcu_prep_idle("Callbacks drained");
 }
 
-#endif /* #else #if !defined(CONFIG_RCU_FAST_NO_HZ) */
 
 #ifdef CONFIG_RCU_CPU_STALL_INFO
-
-#ifdef CONFIG_RCU_FAST_NO_HZ
-
 static void print_cpu_stall_fast_no_hz(char *cp, int cpu)
 {
 	struct hrtimer *hrtp = &per_cpu(rcu_idle_gp_timer, cpu);
@@ -2201,14 +2203,11 @@ static void print_cpu_stall_fast_no_hz(char *cp, int cpu)
 			? ktime_to_us(hrtimer_get_remaining(hrtp))
 			: -1);
 }
+#endif /* #ifdef CONFIG_RCU_CPU_STALL_INFO */
 
-#else /* #ifdef CONFIG_RCU_FAST_NO_HZ */
-
-static void print_cpu_stall_fast_no_hz(char *cp, int cpu)
-{
-}
+#endif /* #else #if !defined(CONFIG_RCU_FAST_NO_HZ) */
 
-#endif /* #else #ifdef CONFIG_RCU_FAST_NO_HZ */
+#ifdef CONFIG_RCU_CPU_STALL_INFO
 
 /* Initiate the stall-info list. */
 static void print_cpu_stall_info_begin(void)
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH] rcutree: Fix v3.4-rc2-rt2 build break
@ 2012-04-11 14:42 ` John Kacur
  0 siblings, 0 replies; 7+ messages in thread
From: John Kacur @ 2012-04-11 14:42 UTC (permalink / raw)
  To: Thomas Gleixner, Paul McKenney; +Cc: lkml, rt-users, John Kacur

Fix build break of the following types.

linux-rt/kernel/rcutree_plugin.h: In function ‘print_cpu_stall_fast_no_hz’:
linux-rt/kernel/rcutree_plugin.h:2195: error: ‘rcu_idle_gp_timer’ undeclared (first use in this function)
linux-rt/kernel/rcutree_plugin.h:2195: error: (Each undeclared identifier is reported only once
linux-rt/kernel/rcutree_plugin.h:2195: error: for each function it appears in.)

The build break only occurs with the PREEMPT_RT_FULL patch applied, however
the patch is meant to go upstream and be applied to v3.4-rc2 as well because
it makes the code more legible there, and will reduce the number of places
where #ifdef PREEMPT_RT_FULL is required should that go upstream someday.

Signed-off-by: John Kacur <jkacur@redhat.com>
---
 kernel/rcutree_plugin.h |   19 +++++++++----------
 1 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/kernel/rcutree_plugin.h b/kernel/rcutree_plugin.h
index 14acafc..1db9471 100644
--- a/kernel/rcutree_plugin.h
+++ b/kernel/rcutree_plugin.h
@@ -1938,6 +1938,12 @@ static void rcu_prepare_for_idle(int cpu)
 {
 }
 
+#ifdef CONFIG_RCU_CPU_STALL_INFO
+static void print_cpu_stall_fast_no_hz(char *cp, int cpu)
+{
+}
+#endif /* #ifdef CONFIG_RCU_CPU_STALL_INFO */
+
 #else /* #if !defined(CONFIG_RCU_FAST_NO_HZ) */
 
 /*
@@ -2184,12 +2190,8 @@ static void rcu_prepare_for_idle(int cpu)
 		trace_rcu_prep_idle("Callbacks drained");
 }
 
-#endif /* #else #if !defined(CONFIG_RCU_FAST_NO_HZ) */
 
 #ifdef CONFIG_RCU_CPU_STALL_INFO
-
-#ifdef CONFIG_RCU_FAST_NO_HZ
-
 static void print_cpu_stall_fast_no_hz(char *cp, int cpu)
 {
 	struct hrtimer *hrtp = &per_cpu(rcu_idle_gp_timer, cpu);
@@ -2201,14 +2203,11 @@ static void print_cpu_stall_fast_no_hz(char *cp, int cpu)
 			? ktime_to_us(hrtimer_get_remaining(hrtp))
 			: -1);
 }
+#endif /* #ifdef CONFIG_RCU_CPU_STALL_INFO */
 
-#else /* #ifdef CONFIG_RCU_FAST_NO_HZ */

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] rcutree: Fix v3.4-rc2-rt2 build break
  2012-04-11 14:42 ` John Kacur
  (?)
@ 2012-04-12 19:46 ` Thomas Gleixner
  2012-04-12 23:11     ` Paul E. McKenney
  2012-04-12 23:13     ` John Kacur
  -1 siblings, 2 replies; 7+ messages in thread
From: Thomas Gleixner @ 2012-04-12 19:46 UTC (permalink / raw)
  To: John Kacur; +Cc: Paul McKenney, lkml, rt-users

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1238 bytes --]

On Wed, 11 Apr 2012, John Kacur wrote:

> Fix build break of the following types.
> 
> linux-rt/kernel/rcutree_plugin.h: In function ‘print_cpu_stall_fast_no_hz’:
> linux-rt/kernel/rcutree_plugin.h:2195: error: ‘rcu_idle_gp_timer’ undeclared (first use in this function)
> linux-rt/kernel/rcutree_plugin.h:2195: error: (Each undeclared identifier is reported only once
> linux-rt/kernel/rcutree_plugin.h:2195: error: for each function it appears in.)
> 
> The build break only occurs with the PREEMPT_RT_FULL patch applied, however
> the patch is meant to go upstream and be applied to v3.4-rc2 as well because
> it makes the code more legible there, and will reduce the number of places
> where #ifdef PREEMPT_RT_FULL is required should that go upstream someday.

-ENOPARSE

I really have no idea what the patch is solving and which particular
combination of config items is causing the above.

If you think that the patch should go upstream then please send a
separate one with a changelog which explains the simplifcation.

I agree that this ifdef maze can do with simplification, but providing
a changelog which tells nothing at all does not make it easier to grok
the problem and understand what's simplified.

Thanks,

	tglx

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] rcutree: Fix v3.4-rc2-rt2 build break
  2012-04-12 19:46 ` Thomas Gleixner
@ 2012-04-12 23:11     ` Paul E. McKenney
  2012-04-12 23:13     ` John Kacur
  1 sibling, 0 replies; 7+ messages in thread
From: Paul E. McKenney @ 2012-04-12 23:11 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: John Kacur, lkml, rt-users

On Thu, Apr 12, 2012 at 09:46:16PM +0200, Thomas Gleixner wrote:
> On Wed, 11 Apr 2012, John Kacur wrote:
> 
> > Fix build break of the following types.
> > 
> > linux-rt/kernel/rcutree_plugin.h: In function ‘print_cpu_stall_fast_no_hz’:
> > linux-rt/kernel/rcutree_plugin.h:2195: error: ‘rcu_idle_gp_timer’ undeclared (first use in this function)
> > linux-rt/kernel/rcutree_plugin.h:2195: error: (Each undeclared identifier is reported only once
> > linux-rt/kernel/rcutree_plugin.h:2195: error: for each function it appears in.)
> > 
> > The build break only occurs with the PREEMPT_RT_FULL patch applied, however
> > the patch is meant to go upstream and be applied to v3.4-rc2 as well because
> > it makes the code more legible there, and will reduce the number of places
> > where #ifdef PREEMPT_RT_FULL is required should that go upstream someday.

OK, I will bite...

What is PREEMPT_RT_FULL doing to make rcu_idle_gp_timer, which is under
CONFIG_RCU_FAST_NO_HZ, inaccessible to print_cpu_stall_fast_no_hz() which
is under both CONFIG_RCU_FAST_NO_HZ and CONFIG_RCU_CPU_STALL_INFO?

> -ENOPARSE
> 
> I really have no idea what the patch is solving and which particular
> combination of config items is causing the above.
> 
> If you think that the patch should go upstream then please send a
> separate one with a changelog which explains the simplifcation.
> 
> I agree that this ifdef maze can do with simplification, but providing
> a changelog which tells nothing at all does not make it easier to grok
> the problem and understand what's simplified.

I must confess that I have been considering re-organizing the #ifdefs
in kernel/rcutree_plugin.h so that all variants of a given function are
adjacent, but I have not yet felt quite masochistic enough to insert
quite that many extra #ifdefs.

The current organization is as follows:

o	Boot-time announce functions.

o	A large number of plugin functions that have real code
	for CONFIG_TREE_PREEMPT_RCU and trivial definitions for
	CONFIG_TREE_RCU.  In the 3.3 kernel, this region extends from line
	73 to line 1135, and used to be the whole point of this file.
	This region contains additional #ifdefs for CONFIG_HOTPLUG_CPU,
	CONFIG_RCU_BOOST (which cannot go into the next region because
	they are used in this region) CONFIG_RCU_CPU_STALL_VERBOSE,
	and CONFIG_PROVE_LOCKING.

o	Another set of plugin functions that have real code for
	CONFIG_RCU_BOOST and trivial definitions otherwise.  In the
	3.3 kernel, this region extends from line 1137 to line 1824,
	and contains additional #ifdefs for CONFIG_HOTPLUG_CPU and
	CONFIG_RCU_TRACE.

o	Yet another set of plugin functions that have real code for
	CONFIG_SMP and trivial definitions otherwise.  In the 3.3
	kernel this region extends from line 1826 to 1950.  This
	is going away, as CONFIG_TREE_RCU&!CONFIG_SMP will no longer
	be an option.

o	The next set of plugin functions have real code for
	CONFIG_RCU_FAST_NO_HZ and trivial definitions otherwise.
	In the 3.3 kernel, this region extends from line 1952 to
	line 2199.  This region contains additional #ifdefs for
	CONFIG_TREE_PREEMPT_RCU.

o	A new set of plugin functions is going in for
	CONFIG_RCU_CPU_STALL_INFO.

So that is the organization.  I might contemplate small changes such
as moving a function or two to save a #ifdef, but I would have to be
convinced that any wholesale reorganization would actually produce
better results.  Given the amount of code, don't expect me to be easy
to convince.  ;-)

							Thanx, Paul


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] rcutree: Fix v3.4-rc2-rt2 build break
@ 2012-04-12 23:11     ` Paul E. McKenney
  0 siblings, 0 replies; 7+ messages in thread
From: Paul E. McKenney @ 2012-04-12 23:11 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: John Kacur, lkml, rt-users

On Thu, Apr 12, 2012 at 09:46:16PM +0200, Thomas Gleixner wrote:
> On Wed, 11 Apr 2012, John Kacur wrote:
> 
> > Fix build break of the following types.
> > 
> > linux-rt/kernel/rcutree_plugin.h: In function ‘print_cpu_stall_fast_no_hz’:
> > linux-rt/kernel/rcutree_plugin.h:2195: error: ‘rcu_idle_gp_timer’ undeclared (first use in this function)
> > linux-rt/kernel/rcutree_plugin.h:2195: error: (Each undeclared identifier is reported only once
> > linux-rt/kernel/rcutree_plugin.h:2195: error: for each function it appears in.)
> > 
> > The build break only occurs with the PREEMPT_RT_FULL patch applied, however
> > the patch is meant to go upstream and be applied to v3.4-rc2 as well because
> > it makes the code more legible there, and will reduce the number of places
> > where #ifdef PREEMPT_RT_FULL is required should that go upstream someday.

OK, I will bite...

What is PREEMPT_RT_FULL doing to make rcu_idle_gp_timer, which is under
CONFIG_RCU_FAST_NO_HZ, inaccessible to print_cpu_stall_fast_no_hz() which
is under both CONFIG_RCU_FAST_NO_HZ and CONFIG_RCU_CPU_STALL_INFO?

> -ENOPARSE
> 
> I really have no idea what the patch is solving and which particular
> combination of config items is causing the above.
> 
> If you think that the patch should go upstream then please send a
> separate one with a changelog which explains the simplifcation.
> 
> I agree that this ifdef maze can do with simplification, but providing
> a changelog which tells nothing at all does not make it easier to grok
> the problem and understand what's simplified.

I must confess that I have been considering re-organizing the #ifdefs
in kernel/rcutree_plugin.h so that all variants of a given function are
adjacent, but I have not yet felt quite masochistic enough to insert
quite that many extra #ifdefs.

The current organization is as follows:

o	Boot-time announce functions.

o	A large number of plugin functions that have real code
	for CONFIG_TREE_PREEMPT_RCU and trivial definitions for
	CONFIG_TREE_RCU.  In the 3.3 kernel, this region extends from line
	73 to line 1135, and used to be the whole point of this file.
	This region contains additional #ifdefs for CONFIG_HOTPLUG_CPU,
	CONFIG_RCU_BOOST (which cannot go into the next region because
	they are used in this region) CONFIG_RCU_CPU_STALL_VERBOSE,
	and CONFIG_PROVE_LOCKING.

o	Another set of plugin functions that have real code for
	CONFIG_RCU_BOOST and trivial definitions otherwise.  In the
	3.3 kernel, this region extends from line 1137 to line 1824,
	and contains additional #ifdefs for CONFIG_HOTPLUG_CPU and
	CONFIG_RCU_TRACE.

o	Yet another set of plugin functions that have real code for
	CONFIG_SMP and trivial definitions otherwise.  In the 3.3
	kernel this region extends from line 1826 to 1950.  This
	is going away, as CONFIG_TREE_RCU&!CONFIG_SMP will no longer
	be an option.

o	The next set of plugin functions have real code for
	CONFIG_RCU_FAST_NO_HZ and trivial definitions otherwise.
	In the 3.3 kernel, this region extends from line 1952 to
	line 2199.  This region contains additional #ifdefs for
	CONFIG_TREE_PREEMPT_RCU.

o	A new set of plugin functions is going in for
	CONFIG_RCU_CPU_STALL_INFO.

So that is the organization.  I might contemplate small changes such
as moving a function or two to save a #ifdef, but I would have to be
convinced that any wholesale reorganization would actually produce
better results.  Given the amount of code, don't expect me to be easy
to convince.  ;-)

							Thanx, Paul

--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] rcutree: Fix v3.4-rc2-rt2 build break
  2012-04-12 19:46 ` Thomas Gleixner
@ 2012-04-12 23:13     ` John Kacur
  2012-04-12 23:13     ` John Kacur
  1 sibling, 0 replies; 7+ messages in thread
From: John Kacur @ 2012-04-12 23:13 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: Paul McKenney, lkml, rt-users

On Thu, Apr 12, 2012 at 9:46 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
>
> On Wed, 11 Apr 2012, John Kacur wrote:
>
> > Fix build break of the following types.
> >
> > linux-rt/kernel/rcutree_plugin.h: In function
> > ‘print_cpu_stall_fast_no_hz’:
> > linux-rt/kernel/rcutree_plugin.h:2195: error: ‘rcu_idle_gp_timer’
> > undeclared (first use in this function)
> > linux-rt/kernel/rcutree_plugin.h:2195: error: (Each undeclared
> > identifier is reported only once
> > linux-rt/kernel/rcutree_plugin.h:2195: error: for each function it
> > appears in.)
> >
> > The build break only occurs with the PREEMPT_RT_FULL patch applied,
> > however
> > the patch is meant to go upstream and be applied to v3.4-rc2 as well
> > because
> > it makes the code more legible there, and will reduce the number of
> > places
> > where #ifdef PREEMPT_RT_FULL is required should that go upstream
> > someday.
>
> -ENOPARSE
>
> I really have no idea what the patch is solving and which particular
> combination of config items is causing the above.

The config combination that causes a build break is

CONFIG_RCU_FAST_NO_HZ
CONFIG_PREEMPT_RT_FULL
CONFIG_RCU_CPU_STALL_INFO

The patch does the following

#if !defined(CONFIG_RCU_FAST_NO_HZ) || defined(CONFIG_PREEMPT_RT_FULL
        The patch moves the skeleton versions of the functions to this section
#else
        Versions of the functions that were causing a build break.
#endif

>
> If you think that the patch should go upstream then please send a
> separate one with a changelog which explains the simplifcation.

Applying the patch upstream causes no functional changes there.
I can try resubmitting a patch for upstream with a clearer changelog.
Or I perhaps I should dig deeper and try to simplify more for
upstream, but this is the simplest way I saw to fix the build break
for rt

>
> I agree that this ifdef maze can do with simplification, but providing
> a changelog which tells nothing at all does not make it easier to grok
> the problem and understand what's simplified.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] rcutree: Fix v3.4-rc2-rt2 build break
@ 2012-04-12 23:13     ` John Kacur
  0 siblings, 0 replies; 7+ messages in thread
From: John Kacur @ 2012-04-12 23:13 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: Paul McKenney, lkml, rt-users

On Thu, Apr 12, 2012 at 9:46 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
>
> On Wed, 11 Apr 2012, John Kacur wrote:
>
> > Fix build break of the following types.
> >
> > linux-rt/kernel/rcutree_plugin.h: In function
> > ‘print_cpu_stall_fast_no_hz’:
> > linux-rt/kernel/rcutree_plugin.h:2195: error: ‘rcu_idle_gp_timer’
> > undeclared (first use in this function)
> > linux-rt/kernel/rcutree_plugin.h:2195: error: (Each undeclared
> > identifier is reported only once
> > linux-rt/kernel/rcutree_plugin.h:2195: error: for each function it
> > appears in.)
> >
> > The build break only occurs with the PREEMPT_RT_FULL patch applied,
> > however
> > the patch is meant to go upstream and be applied to v3.4-rc2 as well
> > because
> > it makes the code more legible there, and will reduce the number of
> > places
> > where #ifdef PREEMPT_RT_FULL is required should that go upstream
> > someday.
>
> -ENOPARSE
>
> I really have no idea what the patch is solving and which particular
> combination of config items is causing the above.

The config combination that causes a build break is

CONFIG_RCU_FAST_NO_HZ
CONFIG_PREEMPT_RT_FULL
CONFIG_RCU_CPU_STALL_INFO

The patch does the following

#if !defined(CONFIG_RCU_FAST_NO_HZ) || defined(CONFIG_PREEMPT_RT_FULL
        The patch moves the skeleton versions of the functions to this section
#else
        Versions of the functions that were causing a build break.
#endif

>
> If you think that the patch should go upstream then please send a
> separate one with a changelog which explains the simplifcation.

Applying the patch upstream causes no functional changes there.
I can try resubmitting a patch for upstream with a clearer changelog.
Or I perhaps I should dig deeper and try to simplify more for
upstream, but this is the simplest way I saw to fix the build break
for rt

>
> I agree that this ifdef maze can do with simplification, but providing
> a changelog which tells nothing at all does not make it easier to grok
> the problem and understand what's simplified.
--
To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2012-04-12 23:13 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-11 14:42 [PATCH] rcutree: Fix v3.4-rc2-rt2 build break John Kacur
2012-04-11 14:42 ` John Kacur
2012-04-12 19:46 ` Thomas Gleixner
2012-04-12 23:11   ` Paul E. McKenney
2012-04-12 23:11     ` Paul E. McKenney
2012-04-12 23:13   ` John Kacur
2012-04-12 23:13     ` John Kacur

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.