2009-09-21 06:44:46

by Paul Mackerras

[permalink] [raw]
Subject: [PATCH] perf_counter/powerpc: Fix compilation after perf_counter_overflow change

Commit 5622f295 ("x86, perf_counter, bts: Optimize BTS overflow
handling") removed the regs field from struct perf_sample_data and
added a regs parameter to perf_counter_overflow(). This breaks the
build on powerpc as reported by Sachin Sant:

arch/powerpc/kernel/perf_counter.c: In function 'record_and_restart':
arch/powerpc/kernel/perf_counter.c:1165: error: unknown field 'regs' specified in initializer
cc1: warnings being treated as errors
arch/powerpc/kernel/perf_counter.c:1165: error: initialization makes integer from pointer without a cast
arch/powerpc/kernel/perf_counter.c:1173: error: too few arguments to function 'perf_counter_overflow'
make[1]: *** [arch/powerpc/kernel/perf_counter.o] Error 1
make: *** [arch/powerpc/kernel] Error 2

This adjusts arch/powerpc/kernel/perf_counter.c to correspond with the
new struct perf_sample_data and perf_counter_overflow().

Reported-by: Sachin Sant <[email protected]>
Signed-off-by: Paul Mackerras <[email protected]>
---
I missed this problem when the "x86, perf_counter, bts: Optimize BTS
overflow handling" patch was posted because the headline made it seem
entirely x86-specific, and the changes to struct perf_sample_data and
perf_counter_overflow() were not mentioned in the changelog.

Markus, please take care in future to mention it in the changelog if
your patches touch definitions used by other architectures. If you
could go so far as to use grep a bit more and fix up other
architectures' callsites for the things you're changing, that would be
very much appreciated. Thanks.

diff --git a/arch/powerpc/kernel/perf_counter.c b/arch/powerpc/kernel/perf_counter.c
index 7ceefaf..5ccf9bc 100644
--- a/arch/powerpc/kernel/perf_counter.c
+++ b/arch/powerpc/kernel/perf_counter.c
@@ -1162,7 +1162,6 @@ static void record_and_restart(struct perf_counter *counter, unsigned long val,
*/
if (record) {
struct perf_sample_data data = {
- .regs = regs,
.addr = 0,
.period = counter->hw.last_period,
};
@@ -1170,7 +1169,7 @@ static void record_and_restart(struct perf_counter *counter, unsigned long val,
if (counter->attr.sample_type & PERF_SAMPLE_ADDR)
perf_get_data_addr(regs, &data.addr);

- if (perf_counter_overflow(counter, nmi, &data)) {
+ if (perf_counter_overflow(counter, nmi, &data, regs)) {
/*
* Interrupts are coming too fast - throttle them
* by setting the counter to 0, so it will be


2009-09-21 07:11:41

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] perf_counter/powerpc: Fix compilation after perf_counter_overflow change


* Paul Mackerras <[email protected]> wrote:

> Commit 5622f295 ("x86, perf_counter, bts: Optimize BTS overflow
> handling") removed the regs field from struct perf_sample_data and
> added a regs parameter to perf_counter_overflow(). This breaks the
> build on powerpc as reported by Sachin Sant:
>
> arch/powerpc/kernel/perf_counter.c: In function 'record_and_restart':
> arch/powerpc/kernel/perf_counter.c:1165: error: unknown field 'regs' specified in initializer
> cc1: warnings being treated as errors
> arch/powerpc/kernel/perf_counter.c:1165: error: initialization makes integer from pointer without a cast
> arch/powerpc/kernel/perf_counter.c:1173: error: too few arguments to function 'perf_counter_overflow'
> make[1]: *** [arch/powerpc/kernel/perf_counter.o] Error 1
> make: *** [arch/powerpc/kernel] Error 2
>
> This adjusts arch/powerpc/kernel/perf_counter.c to correspond with the
> new struct perf_sample_data and perf_counter_overflow().
>
> Reported-by: Sachin Sant <[email protected]>
> Signed-off-by: Paul Mackerras <[email protected]>

Applied, thanks Paul.

> ---
>
> I missed this problem when the "x86, perf_counter, bts: Optimize BTS
> overflow handling" patch was posted because the headline made it seem
> entirely x86-specific, and the changes to struct perf_sample_data and
> perf_counter_overflow() were not mentioned in the changelog.
>
> Markus, please take care in future to mention it in the changelog if
> your patches touch definitions used by other architectures. If you
> could go so far as to use grep a bit more and fix up other
> architectures' callsites for the things you're changing, that would be
> very much appreciated. Thanks.

Yes, that should be done in general - still, nothing beats actual
testing.

Paul, you might also want to test the perfcounter bits of -tip on
PowerPC a bit more frequently - this patch was there for 5 days before i
sent it to Linus.

Cross-builds didnt catch it as perfcounters isnt enabled by default in
any of the powerpc defconfigs:

phoenix:~/linux/linux> grep -w CONFIG_PERF_COUNTERS arch/powerpc/configs/*
arch/powerpc/configs/adder875_defconfig:# CONFIG_PERF_COUNTERS is not set
arch/powerpc/configs/c2k_defconfig:# CONFIG_PERF_COUNTERS is not set
arch/powerpc/configs/ep8248e_defconfig:# CONFIG_PERF_COUNTERS is not set
arch/powerpc/configs/ep88xc_defconfig:# CONFIG_PERF_COUNTERS is not set
arch/powerpc/configs/linkstation_defconfig:# CONFIG_PERF_COUNTERS is not set
arch/powerpc/configs/mgcoge_defconfig:# CONFIG_PERF_COUNTERS is not set
arch/powerpc/configs/mgsuvd_defconfig:# CONFIG_PERF_COUNTERS is not set
arch/powerpc/configs/mpc7448_hpc2_defconfig:# CONFIG_PERF_COUNTERS is not set
arch/powerpc/configs/mpc8272_ads_defconfig:# CONFIG_PERF_COUNTERS is not set
arch/powerpc/configs/mpc83xx_defconfig:# CONFIG_PERF_COUNTERS is not set
arch/powerpc/configs/mpc85xx_defconfig:# CONFIG_PERF_COUNTERS is not set
arch/powerpc/configs/mpc85xx_smp_defconfig:# CONFIG_PERF_COUNTERS is not set
arch/powerpc/configs/mpc866_ads_defconfig:# CONFIG_PERF_COUNTERS is not set
arch/powerpc/configs/mpc86xx_defconfig:# CONFIG_PERF_COUNTERS is not set
arch/powerpc/configs/mpc885_ads_defconfig:# CONFIG_PERF_COUNTERS is not set
arch/powerpc/configs/pq2fads_defconfig:# CONFIG_PERF_COUNTERS is not set
arch/powerpc/configs/prpmc2800_defconfig:# CONFIG_PERF_COUNTERS is not set
arch/powerpc/configs/ps3_defconfig:# CONFIG_PERF_COUNTERS is not set
arch/powerpc/configs/storcenter_defconfig:# CONFIG_PERF_COUNTERS is not set

There's not that many PowerPC users so all extra testing help would be
much welcome. Also, enabling them in the powerpc defconfigs would be
helpful as well.

Thanks,

Ingo

2009-09-21 07:12:54

by Metzger, Markus T

[permalink] [raw]
Subject: RE: [PATCH] perf_counter/powerpc: Fix compilation after perf_counter_overflow change

>-----Original Message-----
>From: Paul Mackerras [mailto:[email protected]]
>Sent: Monday, September 21, 2009 8:45 AM


>Markus, please take care in future to mention it in the changelog if
>your patches touch definitions used by other architectures. If you
>could go so far as to use grep a bit more and fix up other
>architectures' callsites for the things you're changing, that would be
>very much appreciated. Thanks.

I'm sorry I missed that.

There's one more place in arch/sparc/.
The below patch should fix it, but I have no means to test it.

Index: b/arch/sparc/kernel/perf_counter.c
===================================================================
--- a/arch/sparc/kernel/perf_counter.c
+++ b/arch/sparc/kernel/perf_counter.c
@@ -493,7 +493,6 @@ static int __kprobes perf_counter_nmi_ha

regs = args->regs;

- data.regs = regs;
data.addr = 0;

cpuc = &__get_cpu_var(cpu_hw_counters);
@@ -513,7 +512,7 @@ static int __kprobes perf_counter_nmi_ha
if (!sparc_perf_counter_set_period(counter, hwc, idx))
continue;

- if (perf_counter_overflow(counter, 1, &data))
+ if (perf_counter_overflow(counter, 1, &data, regs))
sparc_pmu_disable_counter(hwc, idx);
}


thanks and regards,
markus.

---------------------------------------------------------------------
Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen Germany
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Douglas Lusk, Peter Gleissner, Hannes Schwaderer
Registergericht: Muenchen HRB 47456 Ust.-IdNr.
VAT Registration No.: DE129385895
Citibank Frankfurt (BLZ 502 109 00) 600119052

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

2009-09-21 07:19:46

by Paul Mackerras

[permalink] [raw]
Subject: [tip:perfcounters/urgent] perf_counter/powerpc: Fix compilation after perf_counter_overflow() change

Commit-ID: a42418acb5c2872fc43e028118c275c2245b5eba
Gitweb: http://git.kernel.org/tip/a42418acb5c2872fc43e028118c275c2245b5eba
Author: Paul Mackerras <[email protected]>
AuthorDate: Mon, 21 Sep 2009 16:44:32 +1000
Committer: Ingo Molnar <[email protected]>
CommitDate: Mon, 21 Sep 2009 09:03:34 +0200

perf_counter/powerpc: Fix compilation after perf_counter_overflow() change

Commit 5622f295 ("x86, perf_counter, bts: Optimize BTS overflow
handling") removed the regs field from struct perf_sample_data and
added a regs parameter to perf_counter_overflow(). This breaks the
build on powerpc as reported by Sachin Sant:

arch/powerpc/kernel/perf_counter.c: In function 'record_and_restart':
arch/powerpc/kernel/perf_counter.c:1165: error: unknown field 'regs' specified in initializer

This adjusts arch/powerpc/kernel/perf_counter.c to correspond with the
new struct perf_sample_data and perf_counter_overflow().

Reported-by: Sachin Sant <[email protected]>
Signed-off-by: Paul Mackerras <[email protected]>
Cc: Markus Metzger <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Peter Zijlstra <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>


---
arch/powerpc/kernel/perf_counter.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/perf_counter.c b/arch/powerpc/kernel/perf_counter.c
index 7ceefaf..5ccf9bc 100644
--- a/arch/powerpc/kernel/perf_counter.c
+++ b/arch/powerpc/kernel/perf_counter.c
@@ -1162,7 +1162,6 @@ static void record_and_restart(struct perf_counter *counter, unsigned long val,
*/
if (record) {
struct perf_sample_data data = {
- .regs = regs,
.addr = 0,
.period = counter->hw.last_period,
};
@@ -1170,7 +1169,7 @@ static void record_and_restart(struct perf_counter *counter, unsigned long val,
if (counter->attr.sample_type & PERF_SAMPLE_ADDR)
perf_get_data_addr(regs, &data.addr);

- if (perf_counter_overflow(counter, nmi, &data)) {
+ if (perf_counter_overflow(counter, nmi, &data, regs)) {
/*
* Interrupts are coming too fast - throttle them
* by setting the counter to 0, so it will be

2009-09-21 07:22:10

by Paul Mackerras

[permalink] [raw]
Subject: [tip:perfcounters/urgent] perf_counter, powerpc, sparc: Fix compilation after perf_counter_overflow() change

Commit-ID: 4f95aa6518ca8ae46d88be9abb8e5a33b54812e0
Gitweb: http://git.kernel.org/tip/4f95aa6518ca8ae46d88be9abb8e5a33b54812e0
Author: Paul Mackerras <[email protected]>
AuthorDate: Mon, 21 Sep 2009 16:44:32 +1000
Committer: Ingo Molnar <[email protected]>
CommitDate: Mon, 21 Sep 2009 09:17:24 +0200

perf_counter, powerpc, sparc: Fix compilation after perf_counter_overflow() change

Commit 5622f295 ("x86, perf_counter, bts: Optimize BTS overflow
handling") removed the regs field from struct perf_sample_data and
added a regs parameter to perf_counter_overflow(). This breaks the
build on powerpc (and Sparc) as reported by Sachin Sant:

arch/powerpc/kernel/perf_counter.c: In function 'record_and_restart':
arch/powerpc/kernel/perf_counter.c:1165: error: unknown field 'regs' specified in initializer

This adjusts arch/powerpc/kernel/perf_counter.c to correspond with the
new struct perf_sample_data and perf_counter_overflow().

[ v2: also fix Sparc, Markus Metzger <[email protected]> ]

Reported-by: Sachin Sant <[email protected]>
Signed-off-by: Paul Mackerras <[email protected]>
Cc: Markus Metzger <[email protected]>
Cc: David S. Miller <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Peter Zijlstra <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>


---
arch/powerpc/kernel/perf_counter.c | 3 +--
arch/sparc/kernel/perf_counter.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/kernel/perf_counter.c b/arch/powerpc/kernel/perf_counter.c
index 7ceefaf..5ccf9bc 100644
--- a/arch/powerpc/kernel/perf_counter.c
+++ b/arch/powerpc/kernel/perf_counter.c
@@ -1162,7 +1162,6 @@ static void record_and_restart(struct perf_counter *counter, unsigned long val,
*/
if (record) {
struct perf_sample_data data = {
- .regs = regs,
.addr = 0,
.period = counter->hw.last_period,
};
@@ -1170,7 +1169,7 @@ static void record_and_restart(struct perf_counter *counter, unsigned long val,
if (counter->attr.sample_type & PERF_SAMPLE_ADDR)
perf_get_data_addr(regs, &data.addr);

- if (perf_counter_overflow(counter, nmi, &data)) {
+ if (perf_counter_overflow(counter, nmi, &data, regs)) {
/*
* Interrupts are coming too fast - throttle them
* by setting the counter to 0, so it will be
diff --git a/arch/sparc/kernel/perf_counter.c b/arch/sparc/kernel/perf_counter.c
index 09de403..b1265ce 100644
--- a/arch/sparc/kernel/perf_counter.c
+++ b/arch/sparc/kernel/perf_counter.c
@@ -493,7 +493,6 @@ static int __kprobes perf_counter_nmi_handler(struct notifier_block *self,

regs = args->regs;

- data.regs = regs;
data.addr = 0;

cpuc = &__get_cpu_var(cpu_hw_counters);
@@ -513,7 +512,7 @@ static int __kprobes perf_counter_nmi_handler(struct notifier_block *self,
if (!sparc_perf_counter_set_period(counter, hwc, idx))
continue;

- if (perf_counter_overflow(counter, 1, &data))
+ if (perf_counter_overflow(counter, 1, &data, regs))
sparc_pmu_disable_counter(hwc, idx);
}

2009-09-21 07:30:59

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] perf_counter/powerpc: Fix compilation after perf_counter_overflow change


* Metzger, Markus T <[email protected]> wrote:

> >-----Original Message-----
> >From: Paul Mackerras [mailto:[email protected]]
> >Sent: Monday, September 21, 2009 8:45 AM
>
>
> >Markus, please take care in future to mention it in the changelog if
> >your patches touch definitions used by other architectures. If you
> >could go so far as to use grep a bit more and fix up other
> >architectures' callsites for the things you're changing, that would be
> >very much appreciated. Thanks.
>
> I'm sorry I missed that.
>
> There's one more place in arch/sparc/.
> The below patch should fix it, but I have no means to test it.

You also missed a third thing:

+static inline int
+perf_output_begin(struct perf_output_handle *handle, struct perf_counter *c,
+ unsigned int size, int nmi, int sample) { }

an 'int' function returning void ...

Plus all the !PERF_COUNTERS branch of empty inlines is pointless - these
facilities are used by perfcounters code only. I fixed that too.

>
> Index: b/arch/sparc/kernel/perf_counter.c
> ===================================================================
> --- a/arch/sparc/kernel/perf_counter.c
> +++ b/arch/sparc/kernel/perf_counter.c
> @@ -493,7 +493,6 @@ static int __kprobes perf_counter_nmi_ha
>
> regs = args->regs;
>
> - data.regs = regs;
> data.addr = 0;
>
> cpuc = &__get_cpu_var(cpu_hw_counters);
> @@ -513,7 +512,7 @@ static int __kprobes perf_counter_nmi_ha
> if (!sparc_perf_counter_set_period(counter, hwc, idx))
> continue;
>
> - if (perf_counter_overflow(counter, 1, &data))
> + if (perf_counter_overflow(counter, 1, &data, regs))
> sparc_pmu_disable_counter(hwc, idx);
> }

Looks correct to me and i've also done a Sparc cross build with the fix
in place and it builds fine besides the unrelated build error pasted
below. I've added it to the other fix and if David acks it will send it
to Linus later today.

Thanks,

Ingo

/home/mingo/tip/drivers/video/console/vgacon.c: In function 'vgacon_startup':
/home/mingo/tip/drivers/video/console/vgacon.c:516: warning: passing argument 1 of 'scr_readw' discards qualifiers from pointer target type
/home/mingo/tip/drivers/video/console/vgacon.c:517: warning: passing argument 1 of 'scr_readw' discards qualifiers from pointer target type
/home/mingo/tip/drivers/video/console/vgacon.c:518: warning: passing argument 2 of 'scr_writew' discards qualifiers from pointer target type
/home/mingo/tip/drivers/video/console/vgacon.c:519: warning: passing argument 2 of 'scr_writew' discards qualifiers from pointer target type
/home/mingo/tip/drivers/video/console/vgacon.c:520: warning: passing argument 1 of 'scr_readw' discards qualifiers from pointer target type
/home/mingo/tip/drivers/video/console/vgacon.c:520: warning: passing argument 1 of 'scr_readw' discards qualifiers from pointer target type
/home/mingo/tip/drivers/video/console/vgacon.c:521: warning: passing argument 2 of 'scr_writew' discards qualifiers from pointer target type
/home/mingo/tip/drivers/video/console/vgacon.c:522: warning: passing argument 2 of 'scr_writew' discards qualifiers from pointer target type
/home/mingo/tip/drivers/video/console/vgacon.c:525: warning: passing argument 2 of 'scr_writew' discards qualifiers from pointer target type
/home/mingo/tip/drivers/video/console/vgacon.c:526: warning: passing argument 2 of 'scr_writew' discards qualifiers from pointer target type
/home/mingo/tip/drivers/video/console/vgacon.c:527: warning: passing argument 1 of 'scr_readw' discards qualifiers from pointer target type
/home/mingo/tip/drivers/video/console/vgacon.c:527: warning: passing argument 1 of 'scr_readw' discards qualifiers from pointer target type
/home/mingo/tip/drivers/video/console/vgacon.c:528: warning: passing argument 2 of 'scr_writew' discards qualifiers from pointer target type
/home/mingo/tip/drivers/video/console/vgacon.c:529: warning: passing argument 2 of 'scr_writew' discards qualifiers from pointer target type
/home/mingo/tip/drivers/video/console/vgacon.c:532: warning: passing argument 2 of 'scr_writew' discards qualifiers from pointer target type
/home/mingo/tip/drivers/video/console/vgacon.c:533: warning: passing argument 2 of 'scr_writew' discards qualifiers from pointer target type
/home/mingo/tip/drivers/video/console/vgacon.c: In function 'vgacon_do_font_op':
/home/mingo/tip/drivers/video/console/vgacon.c:1126: error: implicit declaration of function 'vga_writeb'
/home/mingo/tip/drivers/video/console/vgacon.c:1129: error: implicit declaration of function 'vga_readb'
make[4]: *** [drivers/video/console/vgacon.o] Error 1
make[3]: *** [drivers/video/console] Error 2
make[2]: *** [drivers/video] Error 2
make[2]: *** Waiting for unfinished jobs....

2009-09-21 07:34:20

by Paul Mackerras

[permalink] [raw]
Subject: [tip:perfcounters/urgent] perf_counter, powerpc, sparc: Fix compilation after perf_counter_overflow() change

Commit-ID: cd74c86bdf705f824d494a2bbda393d1d562b40a
Gitweb: http://git.kernel.org/tip/cd74c86bdf705f824d494a2bbda393d1d562b40a
Author: Paul Mackerras <[email protected]>
AuthorDate: Mon, 21 Sep 2009 16:44:32 +1000
Committer: Ingo Molnar <[email protected]>
CommitDate: Mon, 21 Sep 2009 09:28:40 +0200

perf_counter, powerpc, sparc: Fix compilation after perf_counter_overflow() change

Commit 5622f295 ("x86, perf_counter, bts: Optimize BTS overflow
handling") removed the regs field from struct perf_sample_data and
added a regs parameter to perf_counter_overflow(). This breaks the
build on powerpc (and Sparc) as reported by Sachin Sant:

arch/powerpc/kernel/perf_counter.c: In function 'record_and_restart':
arch/powerpc/kernel/perf_counter.c:1165: error: unknown field 'regs' specified in initializer

This adjusts arch/powerpc/kernel/perf_counter.c to correspond with the
new struct perf_sample_data and perf_counter_overflow().

[ v2: also fix Sparc, Markus Metzger <[email protected]> ]

Reported-by: Sachin Sant <[email protected]>
Signed-off-by: Paul Mackerras <[email protected]>
Cc: Markus Metzger <[email protected]>
Cc: David S. Miller <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Peter Zijlstra <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>


---
arch/powerpc/kernel/perf_counter.c | 3 +--
arch/sparc/kernel/perf_counter.c | 3 +--
include/linux/perf_counter.h | 17 -----------------
3 files changed, 2 insertions(+), 21 deletions(-)

diff --git a/arch/powerpc/kernel/perf_counter.c b/arch/powerpc/kernel/perf_counter.c
index 7ceefaf..5ccf9bc 100644
--- a/arch/powerpc/kernel/perf_counter.c
+++ b/arch/powerpc/kernel/perf_counter.c
@@ -1162,7 +1162,6 @@ static void record_and_restart(struct perf_counter *counter, unsigned long val,
*/
if (record) {
struct perf_sample_data data = {
- .regs = regs,
.addr = 0,
.period = counter->hw.last_period,
};
@@ -1170,7 +1169,7 @@ static void record_and_restart(struct perf_counter *counter, unsigned long val,
if (counter->attr.sample_type & PERF_SAMPLE_ADDR)
perf_get_data_addr(regs, &data.addr);

- if (perf_counter_overflow(counter, nmi, &data)) {
+ if (perf_counter_overflow(counter, nmi, &data, regs)) {
/*
* Interrupts are coming too fast - throttle them
* by setting the counter to 0, so it will be
diff --git a/arch/sparc/kernel/perf_counter.c b/arch/sparc/kernel/perf_counter.c
index 09de403..b1265ce 100644
--- a/arch/sparc/kernel/perf_counter.c
+++ b/arch/sparc/kernel/perf_counter.c
@@ -493,7 +493,6 @@ static int __kprobes perf_counter_nmi_handler(struct notifier_block *self,

regs = args->regs;

- data.regs = regs;
data.addr = 0;

cpuc = &__get_cpu_var(cpu_hw_counters);
@@ -513,7 +512,7 @@ static int __kprobes perf_counter_nmi_handler(struct notifier_block *self,
if (!sparc_perf_counter_set_period(counter, hwc, idx))
continue;

- if (perf_counter_overflow(counter, 1, &data))
+ if (perf_counter_overflow(counter, 1, &data, regs))
sparc_pmu_disable_counter(hwc, idx);
}

diff --git a/include/linux/perf_counter.h b/include/linux/perf_counter.h
index bd34100..740caad 100644
--- a/include/linux/perf_counter.h
+++ b/include/linux/perf_counter.h
@@ -849,23 +849,6 @@ static inline void perf_counter_comm(struct task_struct *tsk) { }
static inline void perf_counter_fork(struct task_struct *tsk) { }
static inline void perf_counter_init(void) { }

-static inline int
-perf_output_begin(struct perf_output_handle *handle, struct perf_counter *c,
- unsigned int size, int nmi, int sample) { }
-static inline void perf_output_end(struct perf_output_handle *handle) { }
-static inline void
-perf_output_copy(struct perf_output_handle *handle,
- const void *buf, unsigned int len) { }
-static inline void
-perf_output_sample(struct perf_output_handle *handle,
- struct perf_event_header *header,
- struct perf_sample_data *data,
- struct perf_counter *counter) { }
-static inline void
-perf_prepare_sample(struct perf_event_header *header,
- struct perf_sample_data *data,
- struct perf_counter *counter,
- struct pt_regs *regs) { }
#endif

#define perf_output_put(handle, x) \

2009-09-21 07:39:14

by Heiko Carstens

[permalink] [raw]
Subject: Re: [PATCH] perf_counter/powerpc: Fix compilation after perf_counter_overflow change

On Mon, Sep 21, 2009 at 09:30:43AM +0200, Ingo Molnar wrote:
>
> * Metzger, Markus T <[email protected]> wrote:
>
> > >-----Original Message-----
> > >From: Paul Mackerras [mailto:[email protected]]
> > >Sent: Monday, September 21, 2009 8:45 AM
> >
> >
> > >Markus, please take care in future to mention it in the changelog if
> > >your patches touch definitions used by other architectures. If you
> > >could go so far as to use grep a bit more and fix up other
> > >architectures' callsites for the things you're changing, that would be
> > >very much appreciated. Thanks.
> >
> > I'm sorry I missed that.
> >
> > There's one more place in arch/sparc/.
> > The below patch should fix it, but I have no means to test it.
>
> You also missed a third thing:
>
> +static inline int
> +perf_output_begin(struct perf_output_handle *handle, struct perf_counter *c,
> + unsigned int size, int nmi, int sample) { }
>
> an 'int' function returning void ...
>
> Plus all the !PERF_COUNTERS branch of empty inlines is pointless - these
> facilities are used by perfcounters code only. I fixed that too.

Hi Ingo,

did you fix all of these warnings for !PERF_COUNTERS?

include/linux/perf_counter.h: In function 'perf_output_begin':
include/linux/perf_counter.h:854: warning: no return statement in function returning non-void
include/linux/perf_counter.h: At top level:
include/linux/perf_counter.h:863: warning: 'struct perf_sample_data' declared inside parameter list
include/linux/perf_counter.h:863: warning: its scope is only this definition or declaration, which is probably not what you want
include/linux/perf_counter.h:868: warning: 'struct perf_sample_data' declared inside parameter list

2009-09-21 07:45:57

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] perf_counter/powerpc: Fix compilation after perf_counter_overflow change


* Heiko Carstens <[email protected]> wrote:

> On Mon, Sep 21, 2009 at 09:30:43AM +0200, Ingo Molnar wrote:
> >
> > * Metzger, Markus T <[email protected]> wrote:
> >
> > > >-----Original Message-----
> > > >From: Paul Mackerras [mailto:[email protected]]
> > > >Sent: Monday, September 21, 2009 8:45 AM
> > >
> > >
> > > >Markus, please take care in future to mention it in the changelog if
> > > >your patches touch definitions used by other architectures. If you
> > > >could go so far as to use grep a bit more and fix up other
> > > >architectures' callsites for the things you're changing, that would be
> > > >very much appreciated. Thanks.
> > >
> > > I'm sorry I missed that.
> > >
> > > There's one more place in arch/sparc/.
> > > The below patch should fix it, but I have no means to test it.
> >
> > You also missed a third thing:
> >
> > +static inline int
> > +perf_output_begin(struct perf_output_handle *handle, struct perf_counter *c,
> > + unsigned int size, int nmi, int sample) { }
> >
> > an 'int' function returning void ...
> >
> > Plus all the !PERF_COUNTERS branch of empty inlines is pointless - these
> > facilities are used by perfcounters code only. I fixed that too.
>
> Hi Ingo,
>
> did you fix all of these warnings for !PERF_COUNTERS?
>
> include/linux/perf_counter.h: In function 'perf_output_begin':
> include/linux/perf_counter.h:854: warning: no return statement in function returning non-void
> include/linux/perf_counter.h: At top level:
> include/linux/perf_counter.h:863: warning: 'struct perf_sample_data' declared inside parameter list
> include/linux/perf_counter.h:863: warning: its scope is only this definition or declaration, which is probably not what you want
> include/linux/perf_counter.h:868: warning: 'struct perf_sample_data' declared inside parameter list

Yes. The full commit is below.

Ingo

---------------->
>From cd74c86bdf705f824d494a2bbda393d1d562b40a Mon Sep 17 00:00:00 2001
From: Paul Mackerras <[email protected]>
Date: Mon, 21 Sep 2009 16:44:32 +1000
Subject: [PATCH] perf_counter, powerpc, sparc: Fix compilation after perf_counter_overflow() change

Commit 5622f295 ("x86, perf_counter, bts: Optimize BTS overflow
handling") removed the regs field from struct perf_sample_data and
added a regs parameter to perf_counter_overflow(). This breaks the
build on powerpc (and Sparc) as reported by Sachin Sant:

arch/powerpc/kernel/perf_counter.c: In function 'record_and_restart':
arch/powerpc/kernel/perf_counter.c:1165: error: unknown field 'regs' specified in initializer

This adjusts arch/powerpc/kernel/perf_counter.c to correspond with the
new struct perf_sample_data and perf_counter_overflow().

[ v2: also fix Sparc, Markus Metzger <[email protected]> ]

Reported-by: Sachin Sant <[email protected]>
Signed-off-by: Paul Mackerras <[email protected]>
Cc: Markus Metzger <[email protected]>
Cc: David S. Miller <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Peter Zijlstra <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/powerpc/kernel/perf_counter.c | 3 +--
arch/sparc/kernel/perf_counter.c | 3 +--
include/linux/perf_counter.h | 17 -----------------
3 files changed, 2 insertions(+), 21 deletions(-)

diff --git a/arch/powerpc/kernel/perf_counter.c b/arch/powerpc/kernel/perf_counter.c
index 7ceefaf..5ccf9bc 100644
--- a/arch/powerpc/kernel/perf_counter.c
+++ b/arch/powerpc/kernel/perf_counter.c
@@ -1162,7 +1162,6 @@ static void record_and_restart(struct perf_counter *counter, unsigned long val,
*/
if (record) {
struct perf_sample_data data = {
- .regs = regs,
.addr = 0,
.period = counter->hw.last_period,
};
@@ -1170,7 +1169,7 @@ static void record_and_restart(struct perf_counter *counter, unsigned long val,
if (counter->attr.sample_type & PERF_SAMPLE_ADDR)
perf_get_data_addr(regs, &data.addr);

- if (perf_counter_overflow(counter, nmi, &data)) {
+ if (perf_counter_overflow(counter, nmi, &data, regs)) {
/*
* Interrupts are coming too fast - throttle them
* by setting the counter to 0, so it will be
diff --git a/arch/sparc/kernel/perf_counter.c b/arch/sparc/kernel/perf_counter.c
index 09de403..b1265ce 100644
--- a/arch/sparc/kernel/perf_counter.c
+++ b/arch/sparc/kernel/perf_counter.c
@@ -493,7 +493,6 @@ static int __kprobes perf_counter_nmi_handler(struct notifier_block *self,

regs = args->regs;

- data.regs = regs;
data.addr = 0;

cpuc = &__get_cpu_var(cpu_hw_counters);
@@ -513,7 +512,7 @@ static int __kprobes perf_counter_nmi_handler(struct notifier_block *self,
if (!sparc_perf_counter_set_period(counter, hwc, idx))
continue;

- if (perf_counter_overflow(counter, 1, &data))
+ if (perf_counter_overflow(counter, 1, &data, regs))
sparc_pmu_disable_counter(hwc, idx);
}

diff --git a/include/linux/perf_counter.h b/include/linux/perf_counter.h
index bd34100..740caad 100644
--- a/include/linux/perf_counter.h
+++ b/include/linux/perf_counter.h
@@ -849,23 +849,6 @@ static inline void perf_counter_comm(struct task_struct *tsk) { }
static inline void perf_counter_fork(struct task_struct *tsk) { }
static inline void perf_counter_init(void) { }

-static inline int
-perf_output_begin(struct perf_output_handle *handle, struct perf_counter *c,
- unsigned int size, int nmi, int sample) { }
-static inline void perf_output_end(struct perf_output_handle *handle) { }
-static inline void
-perf_output_copy(struct perf_output_handle *handle,
- const void *buf, unsigned int len) { }
-static inline void
-perf_output_sample(struct perf_output_handle *handle,
- struct perf_event_header *header,
- struct perf_sample_data *data,
- struct perf_counter *counter) { }
-static inline void
-perf_prepare_sample(struct perf_event_header *header,
- struct perf_sample_data *data,
- struct perf_counter *counter,
- struct pt_regs *regs) { }
#endif

#define perf_output_put(handle, x) \

2009-09-21 11:19:36

by Paul Mackerras

[permalink] [raw]
Subject: Re: [PATCH] perf_counter/powerpc: Fix compilation after perf_counter_overflow change

Ingo Molnar writes:

> Paul, you might also want to test the perfcounter bits of -tip on
> PowerPC a bit more frequently - this patch was there for 5 days before i
> sent it to Linus.

Yes, I'll try to do that in future.

I hope I didn't come across as blaming anyone for anything - that
wasn't my intention at all.

> Cross-builds didnt catch it as perfcounters isnt enabled by default in
> any of the powerpc defconfigs:

I'll get that fixed too.

Paul.