Re: [PATCH] [alpha] Add minimal support for software performanceevents.

From: Michael Cree
Date: Tue Oct 27 2009 - 04:10:03 EST


This is a multi-part message in MIME format.Ingo Molnar wrote:
* Michael Cree <mcree@xxxxxxxxxxxx> 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 <mcree@xxxxxxxxxxxx>
---
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.