linux-next: percpu/tip tree build failure

linux-next: percpu/tip tree build failure

[Stephen Rothwell]

Hi all,

Today's linux-next build (x86_64 allmodconfig) failed like this:

kernel/hw_breakpoint.c:86: error: 'task_bp_pinned' redeclared as different kind of symbol
kernel/hw_breakpoint.c:55: note: previous declaration of 'task_bp_pinned' was here
kernel/hw_breakpoint.c: In function 'toggle_bp_task_slot':
kernel/hw_breakpoint.c:165: warning: assignment from incompatible pointer type

Caused by commit dd17c8f72993f9461e9c19250e3f155d6d99df22 ("percpu:
remove per_cpu__ prefix") from the percpu tree interacting with commit
56053170ea2a2c0dc17420e9b94aa3ca51d80408 ("hw-breakpoints: Fix task-bound
breakpoint slot allocation") from the tip tree.

I have applied the following patch for today (and can carry it as
necessary:

From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Tue, 8 Dec 2009 18:19:48 +1100
Subject: [PATCH] percpu: make per_cpu variable name unique in hw_breakpoint.c

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
 kernel/hw_breakpoint.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/kernel/hw_breakpoint.c b/kernel/hw_breakpoint.c
index 02b4925..03a0773 100644
--- a/kernel/hw_breakpoint.c
+++ b/kernel/hw_breakpoint.c
@@ -52,7 +52,7 @@
 static DEFINE_PER_CPU(unsigned int, nr_cpu_bp_pinned);
 
 /* Number of pinned task breakpoints in a cpu */
-static DEFINE_PER_CPU(unsigned int, task_bp_pinned[HBP_NUM]);
+static DEFINE_PER_CPU(unsigned int, nr_task_bp_pinned[HBP_NUM]);
 
 /* Number of non-pinned cpu/task breakpoints in a cpu */
 static DEFINE_PER_CPU(unsigned int, nr_bp_flexible);
@@ -73,7 +73,7 @@ static DEFINE_MUTEX(nr_bp_mutex);
 static unsigned int max_task_bp_pinned(int cpu)
 {
 	int i;
-	unsigned int *tsk_pinned = per_cpu(task_bp_pinned, cpu);
+	unsigned int *tsk_pinned = per_cpu(nr_task_bp_pinned, cpu);
 
 	for (i = HBP_NUM -1; i >= 0; i--) {
 		if (tsk_pinned[i] > 0)
@@ -162,7 +162,7 @@ static void toggle_bp_task_slot(struct task_struct *tsk, int cpu, bool enable)
 
 	count = task_bp_pinned(tsk);
 
-	tsk_pinned = per_cpu(task_bp_pinned, cpu);
+	tsk_pinned = per_cpu(nr_task_bp_pinned, cpu);
 	if (enable) {
 		tsk_pinned[count]++;
 		if (count > 0)
@@ -209,7 +209,7 @@ static void toggle_bp_slot(struct perf_event *bp, bool enable)
  *   - If attached to a single cpu, check:
  *
  *       (per_cpu(nr_bp_flexible, cpu) || (per_cpu(nr_cpu_bp_pinned, cpu)
- *           + max(per_cpu(task_bp_pinned, cpu)))) < HBP_NUM
+ *           + max(per_cpu(nr_task_bp_pinned, cpu)))) < HBP_NUM
  *
  *       -> If there are already non-pinned counters in this cpu, it means
  *          there is already a free slot for them.
@@ -220,7 +220,7 @@ static void toggle_bp_slot(struct perf_event *bp, bool enable)
  *   - If attached to every cpus, check:
  *
  *       (per_cpu(nr_bp_flexible, *) || (max(per_cpu(nr_cpu_bp_pinned, *))
- *           + max(per_cpu(task_bp_pinned, *)))) < HBP_NUM
+ *           + max(per_cpu(nr_task_bp_pinned, *)))) < HBP_NUM
  *
  *       -> This is roughly the same, except we check the number of per cpu
  *          bp for every cpu and we keep the max one. Same for the per tasks
@@ -232,7 +232,7 @@ static void toggle_bp_slot(struct perf_event *bp, bool enable)
  *   - If attached to a single cpu, check:
  *
  *       ((per_cpu(nr_bp_flexible, cpu) > 1) + per_cpu(nr_cpu_bp_pinned, cpu)
- *            + max(per_cpu(task_bp_pinned, cpu))) < HBP_NUM
+ *            + max(per_cpu(nr_task_bp_pinned, cpu))) < HBP_NUM
  *
  *       -> Same checks as before. But now the nr_bp_flexible, if any, must keep
  *          one register at least (or they will never be fed).
@@ -240,7 +240,7 @@ static void toggle_bp_slot(struct perf_event *bp, bool enable)
  *   - If attached to every cpus, check:
  *
  *       ((per_cpu(nr_bp_flexible, *) > 1) + max(per_cpu(nr_cpu_bp_pinned, *))
- *            + max(per_cpu(task_bp_pinned, *))) < HBP_NUM
+ *            + max(per_cpu(nr_task_bp_pinned, *))) < HBP_NUM
  */
 int reserve_bp_slot(struct perf_event *bp)
 {
-- 
1.6.5.3

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

Re: linux-next: percpu/tip tree build failure

[Frederic Weisbecker]

On Tue, Dec 08, 2009 at 06:25:15PM +1100, Stephen Rothwell wrote:
> Hi all,
> 
> Today's linux-next build (x86_64 allmodconfig) failed like this:
> 
> kernel/hw_breakpoint.c:86: error: 'task_bp_pinned' redeclared as different kind of symbol
> kernel/hw_breakpoint.c:55: note: previous declaration of 'task_bp_pinned' was here
> kernel/hw_breakpoint.c: In function 'toggle_bp_task_slot':
> kernel/hw_breakpoint.c:165: warning: assignment from incompatible pointer type
> 
> Caused by commit dd17c8f72993f9461e9c19250e3f155d6d99df22 ("percpu:
> remove per_cpu__ prefix") from the percpu tree interacting with commit
> 56053170ea2a2c0dc17420e9b94aa3ca51d80408 ("hw-breakpoints: Fix task-bound
> breakpoint slot allocation") from the tip tree.
> 
> I have applied the following patch for today (and can carry it as
> necessary:
> 
> From: Stephen Rothwell <sfr@canb.auug.org.au>
> Date: Tue, 8 Dec 2009 18:19:48 +1100
> Subject: [PATCH] percpu: make per_cpu variable name unique in hw_breakpoint.c
> 
> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>


Doh!

Thanks Stephen.

Ingo, can you queue up this patch?

Thanks.

Acked-by: Frederic Weisbecker <fweisbec@gmail.com>

Re: linux-next: percpu/tip tree build failure

[Ingo Molnar]

* Frederic Weisbecker <fweisbec@gmail.com> wrote:

> On Tue, Dec 08, 2009 at 06:25:15PM +1100, Stephen Rothwell wrote:
> > Hi all,
> > 
> > Today's linux-next build (x86_64 allmodconfig) failed like this:
> > 
> > kernel/hw_breakpoint.c:86: error: 'task_bp_pinned' redeclared as different kind of symbol
> > kernel/hw_breakpoint.c:55: note: previous declaration of 'task_bp_pinned' was here
> > kernel/hw_breakpoint.c: In function 'toggle_bp_task_slot':
> > kernel/hw_breakpoint.c:165: warning: assignment from incompatible pointer type
> > 
> > Caused by commit dd17c8f72993f9461e9c19250e3f155d6d99df22 ("percpu:
> > remove per_cpu__ prefix") from the percpu tree interacting with commit
> > 56053170ea2a2c0dc17420e9b94aa3ca51d80408 ("hw-breakpoints: Fix task-bound
> > breakpoint slot allocation") from the tip tree.
> > 
> > I have applied the following patch for today (and can carry it as
> > necessary:
> > 
> > From: Stephen Rothwell <sfr@canb.auug.org.au>
> > Date: Tue, 8 Dec 2009 18:19:48 +1100
> > Subject: [PATCH] percpu: make per_cpu variable name unique in hw_breakpoint.c
> > 
> > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> 
> 
> Doh!
> 
> Thanks Stephen.
> 
> Ingo, can you queue up this patch?
> 
> Thanks.
> 
> Acked-by: Frederic Weisbecker <fweisbec@gmail.com>

I have applied it - but really, the new percpu namespace changes headed 
towards upstream are quite a nuisance IMO. The 3-4 (trivial to solve) 
breakages i've seen so far affecting code i maintain give us an 
estimation about the ongoing maintainence cost - which wont be high but 
not zero either.

The change that was forced here:

 -static DEFINE_PER_CPU(unsigned int, task_bp_pinned[HBP_NUM]);
 +static DEFINE_PER_CPU(unsigned int, nr_task_bp_pinned[HBP_NUM]);

Is it really an improvement to the old code?

Dunno.

	Ingo

Re: linux-next: percpu/tip tree build failure

[Tejun Heo]

Hello,

On 12/08/2009 05:24 PM, Ingo Molnar wrote:
>> Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
> 
> I have applied it - but really, the new percpu namespace changes headed 
> towards upstream are quite a nuisance IMO. The 3-4 (trivial to solve) 
> breakages i've seen so far affecting code i maintain give us an 
> estimation about the ongoing maintainence cost - which wont be high but 
> not zero either.
> 
> The change that was forced here:
> 
>  -static DEFINE_PER_CPU(unsigned int, task_bp_pinned[HBP_NUM]);
>  +static DEFINE_PER_CPU(unsigned int, nr_task_bp_pinned[HBP_NUM]);
> 
> Is it really an improvement to the old code?
>
> Dunno.

In each specific conflict, I don't think it would be an apparent
improvement but overall I do believe it's headed the right way.  Well,
or, at the very least, I don't see any other viable solution and
you're probably the most strongly affected by the change.  Sorry about
the inconveniences.

I'm waiting for ack for a m68k change before pushing out percpu tree.
I'm not completely determined but I think I'll keep dropping per_cpu__
prefix and sparse annotation in linux-next for one more cycle as
sparse annotation cleanup pass hasn't been done yet.  Once new devel
cycle begins, it might be a good idea to pull in percpu changes into
one of the tip trees so that these nuisances can be detected during
development?

Thanks.

-- 
tejun

[tip:perf/urgent] perf: hw_breakpoints: Fix percpu namespace clash

[tip-bot for Stephen Rothwell]

Commit-ID:  6ab8886326a1b9a3a8d164d8174e3c20703a03a2
Gitweb:     http://git.kernel.org/tip/6ab8886326a1b9a3a8d164d8174e3c20703a03a2
Author:     Stephen Rothwell <sfr@canb.auug.org.au>
AuthorDate: Tue, 8 Dec 2009 18:25:15 +1100
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Tue, 8 Dec 2009 09:34:43 +0100

perf: hw_breakpoints: Fix percpu namespace clash

Today's linux-next build failed with:

  kernel/hw_breakpoint.c:86: error: 'task_bp_pinned' redeclared as different kind of symbol
  ...

Caused by commit dd17c8f72993f9461e9c19250e3f155d6d99df22 ("percpu:
remove per_cpu__ prefix") from the percpu tree interacting with
commit 56053170ea2a2c0dc17420e9b94aa3ca51d80408 ("hw-breakpoints:
Fix task-bound breakpoint slot allocation") from the tip tree.

Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
Acked-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Tejun Heo <tj@kernel.org>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Christoph Lameter <cl@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
LKML-Reference: <20091208182515.bb6dda4a.sfr@canb.auug.org.au>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/hw_breakpoint.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/kernel/hw_breakpoint.c b/kernel/hw_breakpoint.c
index 02b4925..03a0773 100644
--- a/kernel/hw_breakpoint.c
+++ b/kernel/hw_breakpoint.c
@@ -52,7 +52,7 @@
 static DEFINE_PER_CPU(unsigned int, nr_cpu_bp_pinned);
 
 /* Number of pinned task breakpoints in a cpu */
-static DEFINE_PER_CPU(unsigned int, task_bp_pinned[HBP_NUM]);
+static DEFINE_PER_CPU(unsigned int, nr_task_bp_pinned[HBP_NUM]);
 
 /* Number of non-pinned cpu/task breakpoints in a cpu */
 static DEFINE_PER_CPU(unsigned int, nr_bp_flexible);
@@ -73,7 +73,7 @@ static DEFINE_MUTEX(nr_bp_mutex);
 static unsigned int max_task_bp_pinned(int cpu)
 {
 	int i;
-	unsigned int *tsk_pinned = per_cpu(task_bp_pinned, cpu);
+	unsigned int *tsk_pinned = per_cpu(nr_task_bp_pinned, cpu);
 
 	for (i = HBP_NUM -1; i >= 0; i--) {
 		if (tsk_pinned[i] > 0)
@@ -162,7 +162,7 @@ static void toggle_bp_task_slot(struct task_struct *tsk, int cpu, bool enable)
 
 	count = task_bp_pinned(tsk);
 
-	tsk_pinned = per_cpu(task_bp_pinned, cpu);
+	tsk_pinned = per_cpu(nr_task_bp_pinned, cpu);
 	if (enable) {
 		tsk_pinned[count]++;
 		if (count > 0)
@@ -209,7 +209,7 @@ static void toggle_bp_slot(struct perf_event *bp, bool enable)
  *   - If attached to a single cpu, check:
  *
  *       (per_cpu(nr_bp_flexible, cpu) || (per_cpu(nr_cpu_bp_pinned, cpu)
- *           + max(per_cpu(task_bp_pinned, cpu)))) < HBP_NUM
+ *           + max(per_cpu(nr_task_bp_pinned, cpu)))) < HBP_NUM
  *
  *       -> If there are already non-pinned counters in this cpu, it means
  *          there is already a free slot for them.
@@ -220,7 +220,7 @@ static void toggle_bp_slot(struct perf_event *bp, bool enable)
  *   - If attached to every cpus, check:
  *
  *       (per_cpu(nr_bp_flexible, *) || (max(per_cpu(nr_cpu_bp_pinned, *))
- *           + max(per_cpu(task_bp_pinned, *)))) < HBP_NUM
+ *           + max(per_cpu(nr_task_bp_pinned, *)))) < HBP_NUM
  *
  *       -> This is roughly the same, except we check the number of per cpu
  *          bp for every cpu and we keep the max one. Same for the per tasks
@@ -232,7 +232,7 @@ static void toggle_bp_slot(struct perf_event *bp, bool enable)
  *   - If attached to a single cpu, check:
  *
  *       ((per_cpu(nr_bp_flexible, cpu) > 1) + per_cpu(nr_cpu_bp_pinned, cpu)
- *            + max(per_cpu(task_bp_pinned, cpu))) < HBP_NUM
+ *            + max(per_cpu(nr_task_bp_pinned, cpu))) < HBP_NUM
  *
  *       -> Same checks as before. But now the nr_bp_flexible, if any, must keep
  *          one register at least (or they will never be fed).
@@ -240,7 +240,7 @@ static void toggle_bp_slot(struct perf_event *bp, bool enable)
  *   - If attached to every cpus, check:
  *
  *       ((per_cpu(nr_bp_flexible, *) > 1) + max(per_cpu(nr_cpu_bp_pinned, *))
- *            + max(per_cpu(task_bp_pinned, *))) < HBP_NUM
+ *            + max(per_cpu(nr_task_bp_pinned, *))) < HBP_NUM
  */
 int reserve_bp_slot(struct perf_event *bp)
 {