Some perf user space improvements

Some perf user space improvements

[Andi Kleen]

These two patches fix two problems in the "perf" tool I ran  
into recently. The main one is fixing the compilation on gcc 4.0,
which one of my test boxes still uses (that's actually
an old patch submitted some time ago, but never got applied)

-Andi

[PATCH 1/2] perf: Fix compilation on gcc 4.0.2

[Andi Kleen]

From: Andi Kleen <ak@linux.intel.com>

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/Makefile                    |    4 ++--
 tools/perf/util/include/linux/bitops.h |    1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index d1db0f6..f4f6478 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -201,13 +201,13 @@ EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wshadow
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Winit-self
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wpacked
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wredundant-decls
-EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wstack-protector
+#EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wstack-protector
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wstrict-aliasing=3
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wswitch-default
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wswitch-enum
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wno-system-headers
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wundef
-EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wvolatile-register-var
+#EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wvolatile-register-var
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wwrite-strings
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wbad-function-cast
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wmissing-declarations
diff --git a/tools/perf/util/include/linux/bitops.h b/tools/perf/util/include/linux/bitops.h
index bb4ac2e..305f1c2 100644
--- a/tools/perf/util/include/linux/bitops.h
+++ b/tools/perf/util/include/linux/bitops.h
@@ -2,6 +2,7 @@
 #define _PERF_LINUX_BITOPS_H_
 
 #include <linux/kernel.h>
+#include <linux/compiler.h>
 #include <asm/hweight.h>
 
 #define BITS_PER_LONG __WORDSIZE
-- 
1.7.1

[PATCH 2/2] perf: Handle perf-versionnumber

[Andi Kleen]

From: Andi Kleen <ak@linux.intel.com>

Sometimes it's useful to have multiple perfs with version number
around. This was defeated by the internal command code. Don't
try to handle a number as a internal command.

Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
 tools/perf/perf.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index cdd6c03..c14c0fa 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -444,8 +444,11 @@ int main(int argc, const char **argv)
 	 *
 	 * So we just directly call the internal command handler, and
 	 * die if that one cannot handle it.
+ 	 *
+ 	 * If there is a number after the - don't do this. Assume it's
+	 * a perf-VERSIONNUMBER
 	 */
-	if (!prefixcmp(cmd, "perf-")) {
+	if (!prefixcmp(cmd, "perf-") && !isdigit(cmd[5])) {
 		cmd += 5;
 		argv[0] = cmd;
 		handle_internal_command(argc, argv);
-- 
1.7.1

Re: [PATCH 1/2] perf: Fix compilation on gcc 4.0.2

[Peter Zijlstra]

On Thu, 2010-11-18 at 11:51 +0100, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---
>  tools/perf/Makefile                    |    4 ++--
>  tools/perf/util/include/linux/bitops.h |    1 +
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> index d1db0f6..f4f6478 100644
> --- a/tools/perf/Makefile
> +++ b/tools/perf/Makefile
> @@ -201,13 +201,13 @@ EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wshadow
>  EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Winit-self
>  EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wpacked
>  EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wredundant-decls
> -EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wstack-protector
> +#EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wstack-protector

Should we be using more of the kernel makefile magic to detect this?

Re: [PATCH 1/2] perf: Fix compilation on gcc 4.0.2

[Thomas Gleixner]

On Thu, 18 Nov 2010, Andi Kleen wrote:

> From: Andi Kleen <ak@linux.intel.com>
> 
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---
>  tools/perf/Makefile                    |    4 ++--
>  tools/perf/util/include/linux/bitops.h |    1 +
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> index d1db0f6..f4f6478 100644
> --- a/tools/perf/Makefile
> +++ b/tools/perf/Makefile
> @@ -201,13 +201,13 @@ EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wshadow
>  EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Winit-self
>  EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wpacked
>  EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wredundant-decls
> -EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wstack-protector
> +#EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wstack-protector

Hmm, can we make that autodetected ?

Thanks,

	tglx

Re: [PATCH 1/2] perf: Fix compilation on gcc 4.0.2

[Andi Kleen]

> Should we be using more of the kernel makefile magic to detect this?

In theory yes, but the two warnings didn't seem to be too useful
to me so it's probably not worth it just for those two.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

Re: [PATCH 1/2] perf: Fix compilation on gcc 4.0.2

[Eric Dumazet]

Le jeudi 18 novembre 2010 à 14:31 +0100, Andi Kleen a écrit :
> > Should we be using more of the kernel makefile magic to detect this?
> 
> In theory yes, but the two warnings didn't seem to be too useful
> to me so it's probably not worth it just for those two.
> 
> -Andi
> 

Is the second case because of the __always_inline in
util/include/linux/bitops.h ?

Not sure why its needed for test_bit() ?

diff --git a/tools/perf/util/include/linux/bitops.h b/tools/perf/util/include/linux/bitops.h
index bb4ac2e..a1459f2 100644
--- a/tools/perf/util/include/linux/bitops.h
+++ b/tools/perf/util/include/linux/bitops.h
@@ -13,7 +13,7 @@ static inline void set_bit(int nr, unsigned long *addr)
 	addr[nr / BITS_PER_LONG] |= 1UL << (nr % BITS_PER_LONG);
 }
 
-static __always_inline int test_bit(unsigned int nr, const unsigned long *addr)
+static inline int test_bit(unsigned int nr, const unsigned long *addr)
 {
 	return ((1UL << (nr % BITS_PER_LONG)) &
 		(((unsigned long *)addr)[nr / BITS_PER_LONG])) != 0;

Re: [PATCH 1/2] perf: Fix compilation on gcc 4.0.2

[Andi Kleen]

On Thu, Nov 18, 2010 at 02:38:09PM +0100, Eric Dumazet wrote:
> Le jeudi 18 novembre 2010 à 14:31 +0100, Andi Kleen a écrit :
> > > Should we be using more of the kernel makefile magic to detect this?
> > 
> > In theory yes, but the two warnings didn't seem to be too useful
> > to me so it's probably not worth it just for those two.
> > 
> > -Andi
> > 
> 
> Is the second case because of the __always_inline in
> util/include/linux/bitops.h ?

I think so.

> 
> Not sure why its needed for test_bit() ?

Because it generates terrible code when not inlined
and apparently some gcc versions of times past didn't
in some cases.

-Andi

[PATCH 1/2] perf: Fix compilation on gcc 4.0.2 v2

[Andi Kleen]

[Updated version, this time using the try-cc command.
I remove the useless -Wvolatile-register-var warning.
I don't think any kernel coder will really do such
a thing in the first place.]

- Fix includes
- Remove useless -Wvolatile-register-var
- Move -Wstack-protector into stack protector test

Signed-off-by: Andi Kleen <ak@linux.intel.com>

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index d1db0f6..993ecdb 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -201,13 +201,11 @@ EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wshadow
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Winit-self
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wpacked
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wredundant-decls
-EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wstack-protector
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wstrict-aliasing=3
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wswitch-default
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wswitch-enum
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wno-system-headers
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wundef
-EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wvolatile-register-var
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wwrite-strings
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wbad-function-cast
 EXTRA_WARNINGS := $(EXTRA_WARNINGS) -Wmissing-declarations
@@ -287,8 +285,8 @@ endif
 
 -include feature-tests.mak
 
-ifeq ($(call try-cc,$(SOURCE_HELLO),-Werror -fstack-protector-all),y)
-	CFLAGS := $(CFLAGS) -fstack-protector-all
+ifeq ($(call try-cc,$(SOURCE_HELLO),-Werror -fstack-protector-all -Wstack-protector),y)
+	CFLAGS := $(CFLAGS) -fstack-protector-all -Wstack-protector
 endif
 
 
diff --git a/tools/perf/util/include/linux/bitops.h b/tools/perf/util/include/linux/bitops.h
index bb4ac2e..50ce182 100644
--- a/tools/perf/util/include/linux/bitops.h
+++ b/tools/perf/util/include/linux/bitops.h
@@ -1,6 +1,7 @@
 #ifndef _PERF_LINUX_BITOPS_H_
 #define _PERF_LINUX_BITOPS_H_
 
+#include <linux/compiler.h>
 #include <linux/kernel.h>
 #include <asm/hweight.h>
 
-- 
ak@linux.intel.com -- Speaking for myself only.