2009-10-26 08:33:14

by Michael Cree

[permalink] [raw]
Subject: [PATCH] [alpha] Add minimal support for software performance events.

In the kernel the patch enables configuration of the perf event
option, adds the perf_event_open syscall, and includes a minimal
architecture specific asm/perf_event.h header file.

For the perf tool the patch implements an Alpha specific section
in the perf.h header file and adjusts options used in the
Makefile to allow compilation on Alpha. The -Wcast-align gives
a "cast increases required alignment of target type" warning for
the list_for_each_entry() macro. The -fstack-protector-all
option generates a "not supported for this target" warning which
with -Werror causes the compiler to abort.

Signed-off-by: Michael Cree <[email protected]>
---
arch/alpha/Kconfig | 1 +
arch/alpha/include/asm/perf_event.h | 9 +++++++++
arch/alpha/include/asm/unistd.h | 3 ++-
arch/alpha/kernel/systbls.S | 1 +
tools/perf/Makefile | 5 ++---
tools/perf/perf.h | 6 ++++++
6 files changed, 21 insertions(+), 4 deletions(-)
create mode 100644 arch/alpha/include/asm/perf_event.h

diff --git a/arch/alpha/Kconfig b/arch/alpha/Kconfig
index 4434481..bd7261e 100644
--- a/arch/alpha/Kconfig
+++ b/arch/alpha/Kconfig
@@ -9,6 +9,7 @@ config ALPHA
select HAVE_IDE
select HAVE_OPROFILE
select HAVE_SYSCALL_WRAPPERS
+ select HAVE_PERF_EVENTS
help
The Alpha is a 64-bit general-purpose processor designed and
marketed by the Digital Equipment Corporation of blessed memory,
diff --git a/arch/alpha/include/asm/perf_event.h b/arch/alpha/include/asm/perf_event.h
new file mode 100644
index 0000000..3bef852
--- /dev/null
+++ b/arch/alpha/include/asm/perf_event.h
@@ -0,0 +1,9 @@
+#ifndef __ASM_ALPHA_PERF_EVENT_H
+#define __ASM_ALPHA_PERF_EVENT_H
+
+/* Alpha only supports software events through this interface. */
+static inline void set_perf_event_pending(void) { }
+
+#define PERF_EVENT_INDEX_OFFSET 0
+
+#endif /* __ASM_ALPHA_PERF_EVENT_H */
diff --git a/arch/alpha/include/asm/unistd.h b/arch/alpha/include/asm/unistd.h
index 17f72b7..414de17 100644
--- a/arch/alpha/include/asm/unistd.h
+++ b/arch/alpha/include/asm/unistd.h
@@ -447,10 +447,11 @@
#define __NR_preadv 489
#define __NR_pwritev 490
#define __NR_rt_tgsigqueueinfo 491
+#define __NR_perf_event_open 492

#ifdef __KERNEL__

-#define NR_SYSCALLS 492
+#define NR_SYSCALLS 493

#define __ARCH_WANT_IPC_PARSE_VERSION
#define __ARCH_WANT_OLD_READDIR
diff --git a/arch/alpha/kernel/systbls.S b/arch/alpha/kernel/systbls.S
index 78199b9..63c78e4 100644
--- a/arch/alpha/kernel/systbls.S
+++ b/arch/alpha/kernel/systbls.S
@@ -510,6 +510,7 @@ sys_call_table:
.quad sys_preadv
.quad sys_pwritev /* 490 */
.quad sys_rt_tgsigqueueinfo
+ .quad sys_perf_event_open

.size sys_call_table, . - sys_call_table
.type sys_call_table, @object
diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 742a32e..7940d66 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -177,8 +177,7 @@ endif
# Include saner warnings here, which can catch bugs:
#

-EXTRA_WARNINGS := -Wcast-align
-EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wformat
+EXTRA_WARNINGS := -Wformat
EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wformat-security
EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wformat-y2k
EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wshadow
@@ -201,7 +200,7 @@ EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wold-style-definition
EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wstrict-prototypes
EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wdeclaration-after-statement

-CFLAGS = $(MBITS) -ggdb3 -Wall -Wextra -std=gnu99 -Werror -O6 -fstack-protector-all -D_FORTIFY_SOURCE=2 $(EXTRA_WARNINGS)
+CFLAGS = $(MBITS) -ggdb3 -Wall -Wextra -std=gnu99 -Werror -O6 -D_FORTIFY_SOURCE=2 $(EXTRA_WARNINGS)
LDFLAGS = -lpthread -lrt -lelf -lm
ALL_CFLAGS = $(CFLAGS)
ALL_LDFLAGS = $(LDFLAGS)
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 8cc4623..216bdb2 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -47,6 +47,12 @@
#define cpu_relax() asm volatile("":::"memory")
#endif

+#ifdef __alpha__
+#include "../../arch/alpha/include/asm/unistd.h"
+#define rmb() asm volatile("mb" ::: "memory")
+#define cpu_relax() asm volatile("" ::: "memory")
+#endif
+
#include <time.h>
#include <unistd.h>
#include <sys/types.h>
--
1.6.3.3


2009-10-26 08:49:13

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] [alpha] Add minimal support for software performance events.


* Michael Cree <[email protected]> wrote:

> In the kernel the patch enables configuration of the perf event
> option, adds the perf_event_open syscall, and includes a minimal
> architecture specific asm/perf_event.h header file.
>
> For the perf tool the patch implements an Alpha specific section
> in the perf.h header file and adjusts options used in the
> Makefile to allow compilation on Alpha. The -Wcast-align gives
> a "cast increases required alignment of target type" warning for
> the list_for_each_entry() macro. The -fstack-protector-all
> option generates a "not supported for this target" warning which
> with -Werror causes the compiler to abort.
>
> Signed-off-by: Michael Cree <[email protected]>
> ---
> arch/alpha/Kconfig | 1 +
> arch/alpha/include/asm/perf_event.h | 9 +++++++++
> arch/alpha/include/asm/unistd.h | 3 ++-
> arch/alpha/kernel/systbls.S | 1 +
> tools/perf/Makefile | 5 ++---
> tools/perf/perf.h | 6 ++++++
> 6 files changed, 21 insertions(+), 4 deletions(-)
> create mode 100644 arch/alpha/include/asm/perf_event.h

Nice!

I've picked up the perf.h bit in an independent commit. Is there a tree
for Alpha bits?

This portion:

> --- a/tools/perf/Makefile
> +++ b/tools/perf/Makefile
> @@ -201,7 +200,7 @@ EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wold-style-definition
> EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wstrict-prototypes
> EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wdeclaration-after-statement
>
> -CFLAGS = $(MBITS) -ggdb3 -Wall -Wextra -std=gnu99 -Werror -O6 -fstack-protector-all -D_FORTIFY_SOURCE=2 $(EXTRA_WARNINGS)
> +CFLAGS = $(MBITS) -ggdb3 -Wall -Wextra -std=gnu99 -Werror -O6 -D_FORTIFY_SOURCE=2 $(EXTRA_WARNINGS)
> LDFLAGS = -lpthread -lrt -lelf -lm
> ALL_CFLAGS = $(CFLAGS)
> ALL_LDFLAGS = $(LDFLAGS)

Should be done not by removing the stack-protector build unconditionally
- but by auto-testing whether stackprotector is supported by GCC and
using it if yes.

Examples can be found n arch/x86/Makefile's use of
scripts/gcc-*-has-stack-protector.sh.

Thanks,

Ingo

2009-10-26 11:39:39

by Michael Cree

[permalink] [raw]
Subject: [tip:perf/core] perf tools, Alpha: Add Alpha support to perf.h

Commit-ID: fcd14b3203b538dca04a2b065c774c0b57863eec
Gitweb: http://git.kernel.org/tip/fcd14b3203b538dca04a2b065c774c0b57863eec
Author: Michael Cree <[email protected]>
AuthorDate: Mon, 26 Oct 2009 21:32:06 +1300
Committer: Ingo Molnar <[email protected]>
CommitDate: Mon, 26 Oct 2009 09:45:41 +0100

perf tools, Alpha: Add Alpha support to perf.h

For the perf tool the patch implements an Alpha specific section
in the perf.h header file.

Signed-off-by: Michael Cree <[email protected]>
Cc: Richard Henderson <[email protected]>
Cc: Ivan Kokshaysky <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
LKML-Reference: <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
---
tools/perf/perf.h | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 8cc4623..216bdb2 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -47,6 +47,12 @@
#define cpu_relax() asm volatile("":::"memory")
#endif

+#ifdef __alpha__
+#include "../../arch/alpha/include/asm/unistd.h"
+#define rmb() asm volatile("mb" ::: "memory")
+#define cpu_relax() asm volatile("" ::: "memory")
+#endif
+
#include <time.h>
#include <unistd.h>
#include <sys/types.h>

2009-10-26 12:08:26

by Pekka Enberg

[permalink] [raw]
Subject: Re: [tip:perf/core] perf tools, Alpha: Add Alpha support to perf.h

Hi Ingo,

On Mon, Oct 26, 2009 at 1:38 PM, tip-bot for Michael Cree
<[email protected]> wrote:
> Commit-ID: ?fcd14b3203b538dca04a2b065c774c0b57863eec
> Gitweb: ? ? http://git.kernel.org/tip/fcd14b3203b538dca04a2b065c774c0b57863eec
> Author: ? ? Michael Cree <[email protected]>
> AuthorDate: Mon, 26 Oct 2009 21:32:06 +1300
> Committer: ?Ingo Molnar <[email protected]>
> CommitDate: Mon, 26 Oct 2009 09:45:41 +0100
>
> perf tools, Alpha: Add Alpha support to perf.h
>
> For the perf tool the patch implements an Alpha specific section
> in the perf.h header file.
>
> Signed-off-by: Michael Cree <[email protected]>
> Cc: Richard Henderson <[email protected]>
> Cc: Ivan Kokshaysky <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Paul Mackerras <[email protected]>
> LKML-Reference: <[email protected]>
> Signed-off-by: Ingo Molnar <[email protected]>
> ---
> ?tools/perf/perf.h | ? ?6 ++++++
> ?1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/tools/perf/perf.h b/tools/perf/perf.h
> index 8cc4623..216bdb2 100644
> --- a/tools/perf/perf.h
> +++ b/tools/perf/perf.h
> @@ -47,6 +47,12 @@
> ?#define cpu_relax() ? ?asm volatile("":::"memory")
> ?#endif
>
> +#ifdef __alpha__
> +#include "../../arch/alpha/include/asm/unistd.h"
> +#define rmb() ? ? ? ? ?asm volatile("mb" ::: "memory")
> +#define cpu_relax() ? ?asm volatile("" ::: "memory")
> +#endif

OK, I'll bite. We tell userspace developers not to include kernel
headers. Why is it okay for perf to do it (especially for something
that's in asm)?

Pekka

2009-10-26 12:23:15

by Ingo Molnar

[permalink] [raw]
Subject: Re: [tip:perf/core] perf tools, Alpha: Add Alpha support to perf.h


* Pekka Enberg <[email protected]> wrote:

> Hi Ingo,
>
> On Mon, Oct 26, 2009 at 1:38 PM, tip-bot for Michael Cree
> <[email protected]> wrote:
> > Commit-ID: ?fcd14b3203b538dca04a2b065c774c0b57863eec
> > Gitweb: ? ? http://git.kernel.org/tip/fcd14b3203b538dca04a2b065c774c0b57863eec
> > Author: ? ? Michael Cree <[email protected]>
> > AuthorDate: Mon, 26 Oct 2009 21:32:06 +1300
> > Committer: ?Ingo Molnar <[email protected]>
> > CommitDate: Mon, 26 Oct 2009 09:45:41 +0100
> >
> > perf tools, Alpha: Add Alpha support to perf.h
> >
> > For the perf tool the patch implements an Alpha specific section
> > in the perf.h header file.
> >
> > Signed-off-by: Michael Cree <[email protected]>
> > Cc: Richard Henderson <[email protected]>
> > Cc: Ivan Kokshaysky <[email protected]>
> > Cc: Peter Zijlstra <[email protected]>
> > Cc: Paul Mackerras <[email protected]>
> > LKML-Reference: <[email protected]>
> > Signed-off-by: Ingo Molnar <[email protected]>
> > ---
> > ?tools/perf/perf.h | ? ?6 ++++++
> > ?1 files changed, 6 insertions(+), 0 deletions(-)
> >
> > diff --git a/tools/perf/perf.h b/tools/perf/perf.h
> > index 8cc4623..216bdb2 100644
> > --- a/tools/perf/perf.h
> > +++ b/tools/perf/perf.h
> > @@ -47,6 +47,12 @@
> > ?#define cpu_relax() ? ?asm volatile("":::"memory")
> > ?#endif
> >
> > +#ifdef __alpha__
> > +#include "../../arch/alpha/include/asm/unistd.h"
> > +#define rmb() ? ? ? ? ?asm volatile("mb" ::: "memory")
> > +#define cpu_relax() ? ?asm volatile("" ::: "memory")
> > +#endif
>
> OK, I'll bite. We tell userspace developers not to include kernel
> headers. Why is it okay for perf to do it (especially for something
> that's in asm)?

The main counter-argument against inclusion was always "what if we break
them accidentally". I.e. it can become a semi-ABI - stuff we cannot
change because we cannot change the outside projects. With perf this
cannot occur - it's all in one Git tree and can always be fixed/changed.

Note that we reuse a couple of other facilities in tools/perf as well -
linux/list.h, rbtree.c, etc. - and this is good - you can code perf as
if you were hacking on the kernel! ;-)

Ingo

2009-10-27 08:09:57

by Michael Cree

[permalink] [raw]
Subject: Re: [PATCH] [alpha] Add minimal support for software performance events.

Ingo Molnar wrote:
> * Michael Cree <[email protected]> wrote:
>
>> In the kernel the patch enables configuration of the perf event
>> option, adds the perf_event_open syscall, and includes a minimal
>> architecture specific asm/perf_event.h header file.
>>
>> For the perf tool the patch implements an Alpha specific section
>> in the perf.h header file and adjusts options used in the
>> Makefile to allow compilation on Alpha. The -Wcast-align gives
>> a "cast increases required alignment of target type" warning for
>> the list_for_each_entry() macro. The -fstack-protector-all
>> option generates a "not supported for this target" warning which
>> with -Werror causes the compiler to abort.
>>
>> Signed-off-by: Michael Cree <[email protected]>
>> ---
>> arch/alpha/Kconfig | 1 +
>> arch/alpha/include/asm/perf_event.h | 9 +++++++++
>> arch/alpha/include/asm/unistd.h | 3 ++-
>> arch/alpha/kernel/systbls.S | 1 +
>> tools/perf/Makefile | 5 ++---
>> tools/perf/perf.h | 6 ++++++
>> 6 files changed, 21 insertions(+), 4 deletions(-)
>> create mode 100644 arch/alpha/include/asm/perf_event.h
>>
>
> Nice!
>
> I've picked up the perf.h bit in an independent commit. Is there a tree
> for Alpha bits?
>

Not that I know of. Note also that this patch is on top of the patch
"alpha: Wire up missing/new syscalls" recently posted by Daniele Calore
(http://lkml.org/lkml/2009/10/21/99). Hopefully this and prior patches
get picked up by the Alpha maintainers.

> This portion:
>
>> --- a/tools/perf/Makefile
>> +++ b/tools/perf/Makefile
>> @@ -201,7 +200,7 @@ EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wold-style-definition
>> EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wstrict-prototypes
>> EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wdeclaration-after-statement
>>
>> -CFLAGS = $(MBITS) -ggdb3 -Wall -Wextra -std=gnu99 -Werror -O6 -fstack-protector-all -D_FORTIFY_SOURCE=2 $(EXTRA_WARNINGS)
>> +CFLAGS = $(MBITS) -ggdb3 -Wall -Wextra -std=gnu99 -Werror -O6 -D_FORTIFY_SOURCE=2 $(EXTRA_WARNINGS)
>> LDFLAGS = -lpthread -lrt -lelf -lm
>> ALL_CFLAGS = $(CFLAGS)
>> ALL_LDFLAGS = $(LDFLAGS)
>>
>
> Should be done not by removing the stack-protector build unconditionally
> - but by auto-testing whether stackprotector is supported by GCC and
> using it if yes.
>

Revised patch attached. It includes a test that the compiler doesn't
bomb out with -fstack-protector-all and only adds the option to CFLAGS
if ok. But I have had to put the test below the definition of the macro
CC. This has the side effect of separating the addition of
-fstack-protector-all from the main definitions of CFLAGS and
ALL_CFLAGS, and is not ideal in my opinion. The patch also removes
-Wcast-align (I forgot to say that in the commit message of the patch).

Michael.



Attachments:
0001-Test-fstack-protector-all-compiler-option-for-inclus.patch (1.95 kB)

2009-10-27 18:29:49

by Matt Turner

[permalink] [raw]
Subject: Re: [PATCH] [alpha] Add minimal support for software performance events.

On Mon, Oct 26, 2009 at 4:48 AM, Ingo Molnar <[email protected]> wrote:
> Is there a tree for Alpha bits?

No, but really other than a very few scattered patches we don't have
much activity.

Maybe it's worth creating one at git.kernel.org? I don't think Richard
or Ivan are interested in doing it. Could Michael or I do it without
being maintainers?

Matt

2009-10-28 20:56:23

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [PATCH] [alpha] Add minimal support for software performance events.

On Tue, Oct 27, 2009 at 02:29:52PM -0400, Matt Turner wrote:
> On Mon, Oct 26, 2009 at 4:48 AM, Ingo Molnar <[email protected]> wrote:
> > Is there a tree for Alpha bits?
>
> No, but really other than a very few scattered patches we don't have
> much activity.
>
> Maybe it's worth creating one at git.kernel.org? I don't think Richard
> or Ivan are interested in doing it. Could Michael or I do it without
> being maintainers?
If you start handling alpha related stuff then you are automatically becoming
maintainers.
Officially in MAINTAINERS would preferably be with OK from rth and Ivan.

So do not be shy and create such a tree.

Sam

2009-10-28 20:58:44

by Sam Ravnborg

[permalink] [raw]
Subject: Re: [tip:perf/core] perf tools, Alpha: Add Alpha support to perf.h

> >
> > OK, I'll bite. We tell userspace developers not to include kernel
> > headers. Why is it okay for perf to do it (especially for something
> > that's in asm)?
>
> The main counter-argument against inclusion was always "what if we break
> them accidentally". I.e. it can become a semi-ABI - stuff we cannot
> change because we cannot change the outside projects. With perf this
> cannot occur - it's all in one Git tree and can always be fixed/changed.
>
> Note that we reuse a couple of other facilities in tools/perf as well -
> linux/list.h, rbtree.c, etc. - and this is good - you can code perf as
> if you were hacking on the kernel! ;-)

I see no reasons why perf should not use the exported headers
in the default case. unistd.h from alpha is indeed exported.

If perf then on top of that uses some kernel internal
stuff - then pick it up there.
But having perf in the kernel is not an excuse for avoinding
the exported headers.

Sam

2009-11-08 12:23:05

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] [alpha] Add minimal support for software performance events.


* Michael Cree <[email protected]> wrote:

> > Should be done not by removing the stack-protector build
> > unconditionally - but by auto-testing whether stackprotector is
> > supported by GCC and using it if yes.
>
> Revised patch attached. It includes a test that the compiler doesn't
> bomb out with -fstack-protector-all and only adds the option to CFLAGS
> if ok. But I have had to put the test below the definition of the
> macro CC. This has the side effect of separating the addition of
> -fstack-protector-all from the main definitions of CFLAGS and
> ALL_CFLAGS, and is not ideal in my opinion. The patch also removes
> -Wcast-align (I forgot to say that in the commit message of the
> patch).

Nice, i'll queue this up for Linus.

Your S-O-B line was missing from this second patch - i presume you
intended it to be included, right?

Thanks,

Ingo

2009-11-08 12:27:54

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] [alpha] Add minimal support for software performance events.


* Ingo Molnar <[email protected]> wrote:

>
> * Michael Cree <[email protected]> wrote:
>
> > > Should be done not by removing the stack-protector build
> > > unconditionally - but by auto-testing whether stackprotector is
> > > supported by GCC and using it if yes.
> >
> > Revised patch attached. It includes a test that the compiler doesn't
> > bomb out with -fstack-protector-all and only adds the option to CFLAGS
> > if ok. But I have had to put the test below the definition of the
> > macro CC. This has the side effect of separating the addition of
> > -fstack-protector-all from the main definitions of CFLAGS and
> > ALL_CFLAGS, and is not ideal in my opinion. The patch also removes
> > -Wcast-align (I forgot to say that in the commit message of the
> > patch).
>
> Nice, i'll queue this up for Linus.
>
> Your S-O-B line was missing from this second patch - i presume you
> intended it to be included, right?

Mind resending the patch against latest -tip?

http://people.redhat.com/mingo/tip.git/README

There's been other changes in this area so your patch does not apply
anymore.

Ingo

2009-11-10 21:21:57

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [tip:perf/core] perf tools, Alpha: Add Alpha support to perf.h

On Mon, 2009-10-26 at 14:08 +0200, Pekka Enberg wrote:
> > +#ifdef __alpha__
> > +#include "../../arch/alpha/include/asm/unistd.h"
> > +#define rmb() asm volatile("mb" ::: "memory")
> > +#define cpu_relax() asm volatile("" ::: "memory")
> > +#endif
>
> OK, I'll bite. We tell userspace developers not to include kernel
> headers. Why is it okay for perf to do it (especially for something
> that's in asm)?

The reason we take the explicit arch header is because we need the perf
syscall thingy, which isn't yet in the installed system unistd.h because
that's probably some ancient version.

Once distro's have had perf enabled kernels for long enough that all of
userspace has the syscall bits we can remove it.

2009-11-11 07:43:08

by Michael Cree

[permalink] [raw]
Subject: Re: [PATCH] [alpha] Add minimal support for software performance events.

On Sun, Nov 08, 2009 at 01:27:39PM +0100, Ingo Molnar wrote:
> Mind resending the patch against latest -tip?

Not a problem. Attached.

Cheers
Michael.


Attachments:
(No filename) (154.00 B)
0001-Test-fstack-protector-all-compiler-option-for-inclus.patch (1.56 kB)
Download all attachments

2009-11-11 07:49:26

by Michael Cree

[permalink] [raw]
Subject: [tip:perf/core] perf tools: Test -fstack-protector-all compiler option for inclusion in CFLAGS

Commit-ID: 5d7bdab75cd56d2bdc0986ae5546be3b09fea70a
Gitweb: http://git.kernel.org/tip/5d7bdab75cd56d2bdc0986ae5546be3b09fea70a
Author: Michael Cree <[email protected]>
AuthorDate: Wed, 11 Nov 2009 20:43:03 +1300
Committer: Ingo Molnar <[email protected]>
CommitDate: Wed, 11 Nov 2009 08:46:45 +0100

perf tools: Test -fstack-protector-all compiler option for inclusion in CFLAGS

Some architectures (e.g. Alpha) do not support the
-fstack-protector-all compiler option and the use of the option
with -Werror causes the compiler to abort and the build fails.

Test that the compiler supports -fstack-protector-all before
inclusion in CFLAGS.

Signed-off-by: Michael Cree <[email protected]>
Cc: Richard Henderson <[email protected]>
Cc: Ivan Kokshaysky <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
LKML-Reference: <20091111074302.GA3728@omega>
Signed-off-by: Ingo Molnar <[email protected]>
---
tools/perf/Makefile | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index b9509b1..e6d4272 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -207,7 +207,7 @@ ifndef PERF_DEBUG
CFLAGS_OPTIMIZE = -O6
endif

-CFLAGS = $(MBITS) -ggdb3 -Wall -Wextra -std=gnu99 -Werror $(CFLAGS_OPTIMIZE) -fstack-protector-all -D_FORTIFY_SOURCE=2 $(EXTRA_WARNINGS)
+CFLAGS = $(MBITS) -ggdb3 -Wall -Wextra -std=gnu99 -Werror $(CFLAGS_OPTIMIZE) -D_FORTIFY_SOURCE=2 $(EXTRA_WARNINGS)
LDFLAGS = -lpthread -lrt -lelf -lm
ALL_CFLAGS = $(CFLAGS)
ALL_LDFLAGS = $(LDFLAGS)
@@ -259,6 +259,9 @@ PTHREAD_LIBS = -lpthread
# explicitly what architecture to check for. Fix this up for yours..
SPARSE_FLAGS = -D__BIG_ENDIAN__ -D__powerpc__

+ifeq ($(shell sh -c "echo 'int foo(void) {char X[2]; return 3;}' | $(CC) -x c -c -Werror -fstack-protector-all - -o /dev/null >/dev/null 2>&1 && echo y"), y)
+ CFLAGS := $(CFLAGS) -fstack-protector-all
+endif


### --- END CONFIGURATION SECTION ---

2009-12-01 04:30:13

by Matt Turner

[permalink] [raw]
Subject: Re: [PATCH] [alpha] Add minimal support for software performance events.

On Mon, Oct 26, 2009 at 3:32 AM, Michael Cree <[email protected]> wrote:
> In the kernel the patch enables configuration of the perf event
> option, adds the perf_event_open syscall, and includes a minimal
> architecture specific asm/perf_event.h header file.
>
> For the perf tool the patch implements an Alpha specific section
> in the perf.h header file and adjusts options used in the
> Makefile to allow compilation on Alpha. ?The -Wcast-align gives
> a "cast increases required alignment of target type" warning for
> the list_for_each_entry() macro. The -fstack-protector-all
> option generates a "not supported for this target" warning which
> with -Werror causes the compiler to abort.
>
> Signed-off-by: Michael Cree <[email protected]>
> ---
> ?arch/alpha/Kconfig ? ? ? ? ? ? ? ? ?| ? ?1 +
> ?arch/alpha/include/asm/perf_event.h | ? ?9 +++++++++
> ?arch/alpha/include/asm/unistd.h ? ? | ? ?3 ++-
> ?arch/alpha/kernel/systbls.S ? ? ? ? | ? ?1 +
> ?tools/perf/Makefile ? ? ? ? ? ? ? ? | ? ?5 ++---
> ?tools/perf/perf.h ? ? ? ? ? ? ? ? ? | ? ?6 ++++++
> ?6 files changed, 21 insertions(+), 4 deletions(-)
> ?create mode 100644 arch/alpha/include/asm/perf_event.h
>
> diff --git a/arch/alpha/Kconfig b/arch/alpha/Kconfig
> index 4434481..bd7261e 100644
> --- a/arch/alpha/Kconfig
> +++ b/arch/alpha/Kconfig
> @@ -9,6 +9,7 @@ config ALPHA
> ? ? ? ?select HAVE_IDE
> ? ? ? ?select HAVE_OPROFILE
> ? ? ? ?select HAVE_SYSCALL_WRAPPERS
> + ? ? ? select HAVE_PERF_EVENTS
> ? ? ? ?help
> ? ? ? ? ?The Alpha is a 64-bit general-purpose processor designed and
> ? ? ? ? ?marketed by the Digital Equipment Corporation of blessed memory,
> diff --git a/arch/alpha/include/asm/perf_event.h b/arch/alpha/include/asm/perf_event.h
> new file mode 100644
> index 0000000..3bef852
> --- /dev/null
> +++ b/arch/alpha/include/asm/perf_event.h
> @@ -0,0 +1,9 @@
> +#ifndef __ASM_ALPHA_PERF_EVENT_H
> +#define __ASM_ALPHA_PERF_EVENT_H
> +
> +/* Alpha only supports software events through this interface. */
> +static inline void set_perf_event_pending(void) { }
> +
> +#define PERF_EVENT_INDEX_OFFSET 0
> +
> +#endif /* __ASM_ALPHA_PERF_EVENT_H */
> diff --git a/arch/alpha/include/asm/unistd.h b/arch/alpha/include/asm/unistd.h
> index 17f72b7..414de17 100644
> --- a/arch/alpha/include/asm/unistd.h
> +++ b/arch/alpha/include/asm/unistd.h
> @@ -447,10 +447,11 @@
> ?#define __NR_preadv ? ? ? ? ? ? ? ? ? ?489
> ?#define __NR_pwritev ? ? ? ? ? ? ? ? ? 490
> ?#define __NR_rt_tgsigqueueinfo ? ? ? ? 491
> +#define __NR_perf_event_open ? ? ? ? ? 492
>
> ?#ifdef __KERNEL__
>
> -#define NR_SYSCALLS ? ? ? ? ? ? ? ? ? ?492
> +#define NR_SYSCALLS ? ? ? ? ? ? ? ? ? ?493
>
> ?#define __ARCH_WANT_IPC_PARSE_VERSION
> ?#define __ARCH_WANT_OLD_READDIR
> diff --git a/arch/alpha/kernel/systbls.S b/arch/alpha/kernel/systbls.S
> index 78199b9..63c78e4 100644
> --- a/arch/alpha/kernel/systbls.S
> +++ b/arch/alpha/kernel/systbls.S
> @@ -510,6 +510,7 @@ sys_call_table:
> ? ? ? ?.quad sys_preadv
> ? ? ? ?.quad sys_pwritev ? ? ? ? ? ? ? ? ? ? ? /* 490 */
> ? ? ? ?.quad sys_rt_tgsigqueueinfo
> + ? ? ? .quad sys_perf_event_open
>
> ? ? ? ?.size sys_call_table, . - sys_call_table
> ? ? ? ?.type sys_call_table, @object
> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> index 742a32e..7940d66 100644
> --- a/tools/perf/Makefile
> +++ b/tools/perf/Makefile
> @@ -177,8 +177,7 @@ endif
> ?# Include saner warnings here, which can catch bugs:
> ?#
>
> -EXTRA_WARNINGS := -Wcast-align
> -EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wformat
> +EXTRA_WARNINGS := -Wformat
> ?EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wformat-security
> ?EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wformat-y2k
> ?EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wshadow
> @@ -201,7 +200,7 @@ EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wold-style-definition
> ?EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wstrict-prototypes
> ?EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wdeclaration-after-statement
>
> -CFLAGS = $(MBITS) -ggdb3 -Wall -Wextra -std=gnu99 -Werror -O6 -fstack-protector-all -D_FORTIFY_SOURCE=2 $(EXTRA_WARNINGS)
> +CFLAGS = $(MBITS) -ggdb3 -Wall -Wextra -std=gnu99 -Werror -O6 -D_FORTIFY_SOURCE=2 $(EXTRA_WARNINGS)
> ?LDFLAGS = -lpthread -lrt -lelf -lm
> ?ALL_CFLAGS = $(CFLAGS)
> ?ALL_LDFLAGS = $(LDFLAGS)
> diff --git a/tools/perf/perf.h b/tools/perf/perf.h
> index 8cc4623..216bdb2 100644
> --- a/tools/perf/perf.h
> +++ b/tools/perf/perf.h
> @@ -47,6 +47,12 @@
> ?#define cpu_relax() ? ?asm volatile("":::"memory")
> ?#endif
>
> +#ifdef __alpha__
> +#include "../../arch/alpha/include/asm/unistd.h"
> +#define rmb() ? ? ? ? ?asm volatile("mb" ::: "memory")
> +#define cpu_relax() ? ?asm volatile("" ::: "memory")
> +#endif
> +
> ?#include <time.h>
> ?#include <unistd.h>
> ?#include <sys/types.h>
> --
> 1.6.3.3
>
> --

Please take a look at the attached patch and let me know if it's what
you want pushed. I wasn't sure if the last hunk (the memory barriers)
needed to be included or had already been picked up.

Thanks,
Matt


Attachments:
0001-alpha-Add-minimal-support-for-software-performance-e.patch (3.61 kB)

2009-12-01 09:31:30

by Michael Cree

[permalink] [raw]
Subject: Re: [PATCH] [alpha] Add minimal support for software performance events.

On 01/12/09 17:30, Matt Turner wrote:
> Please take a look at the attached patch and let me know if it's what
> you want pushed. I wasn't sure if the last hunk (the memory barriers)
> needed to be included or had already been picked up.

All the tools/perf stuff, including the memory barriers, have been
picked up by linux-tip. Updated patch with the arch/alpha bits only
attached.

The patch should apply cleanly on top of the "alpha: Wire up missing/new
syscalls" patch which I see you have already applied.

Cheers
Michael.


Attachments:
0001-alpha-Add-minimal-support-for-software-performance.patch (2.44 kB)