[PATCH] perf tools: Fix pthread_attr_setaffinity_np() feature detection on Ubuntu systems

[Ingo Molnar <mingo@kernel.org>]

* Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Hi Ingo,
> 
> 	Please consider pulling,
> 
> - Arnaldo
> 
> The following changes since commit 0afb1704010f60e7ae85aef0f93fc10f2d99761e:
> 
>   Merge tag 'perf-core-for-mingo' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core (2015-02-26 12:25:20 +0100)
> 
> are available in the git repository at:
> 
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-core-for-mingo
> 
> for you to fetch changes up to fefd2d9619de3bf0bf02a8622e9f445c3d19cc3f:
> 
>   perf report: Fix branch stack mode cannot be set (2015-02-27 15:52:42 -0300)
> 
> ----------------------------------------------------------------
> perf/core improvements and fixes:
> 
> User visible:
> 
> - Fix SIGBUS failures due to misaligned accesses in Sparc64 (David Ahern)
> 
> - Fix branch stack mode in 'perf report' (He Kuang)
> 
> - Fix a 'perf probe' operator precedence bug (He Kuang)
> 
> - Fix Support for different binaries with same name in 'perf diff' (Kan Liang)
> 
> - Check kprobes blacklist when adding new events via 'perf probe' (Masami Hiramatsu)
> 
> - Add --purge FILE to remove all caches of FILE in 'perf buildid-cache' (Masami Hiramatsu)
> 
> - Show usage with some incorrect params (Masami Hiramatsu)
> 
> - Add new buildid cache if update target is not cached in 'buildid-cache' (Masami Hiramatsu)
> 
> - Allow listing events with 'tracepoint' prefix in 'perf list' (Yunlong Song)
> 
> - Sort the output of 'perf list' (Yunlong Song)
> 
> - Fix bash completion of 'perf --' (Yunlong Song)
> 
> Developer Zone:
> 
> - Handle strdup() failure path in 'perf probe' (Arnaldo Carvalho de Melo)
> 
> - Fix get_real_path to free allocated memory in error path in 'perf probe' (Masami Hiramatsu)
> 
> - Use pr_debug instead of verbose && pr_info perf buildid-cache (Masami Hiramatsu)
> 
> - Fix building of 'perf data' with some gcc versions due to incorrect array struct
>   entry (Yunlong Song)
> 
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> ----------------------------------------------------------------
> Arnaldo Carvalho de Melo (1):
>       perf probe: Handle strdup() failure
> 
> David Ahern (1):
>       perf trace: Fix SIGBUS failures due to misaligned accesses
> 
> He Kuang (2):
>       perf probe: Fix a precedence bug
>       perf report: Fix branch stack mode cannot be set
> 
> Kan Liang (1):
>       perf diff: Support for different binaries
> 
> Masami Hiramatsu (6):
>       perf probe: Check kprobes blacklist when adding new events
>       perf probe: Fix get_real_path to free allocated memory in error path
>       perf buildid-cache: Add new buildid cache if update target is not cached
>       perf buildid-cache: Add --purge FILE to remove all caches of FILE
>       perf buildid-cache: Use pr_debug instead of verbose && pr_info
>       perf buildid-cache: Show usage with incorrect params
> 
> Yunlong Song (8):
>       perf data: Fix sentinel setting for data_cmds array
>       perf list: Sort the output of 'perf list' to view more clearly
>       perf list: Allow listing events with 'tracepoint' prefix
>       perf list: Avoid confusion of perf output and the next command prompt
>       perf tools: Remove the '--(null)' long_name for --list-opts
>       perf list: Clean up the printing functions of hardware/software events
>       perf list: Extend raw-dump to certain kind of events
>       perf tools: Fix the bash completion problem of 'perf --*'
> 
>  tools/perf/Documentation/perf-buildid-cache.txt |  24 ++-
>  tools/perf/Documentation/perf-diff.txt          |   5 +
>  tools/perf/Documentation/perf-list.txt          |   6 +
>  tools/perf/builtin-buildid-cache.c              |  72 ++++++--
>  tools/perf/builtin-data.c                       |   2 +-
>  tools/perf/builtin-list.c                       |  27 ++-
>  tools/perf/builtin-report.c                     |   2 +-
>  tools/perf/builtin-trace.c                      |  36 +++-
>  tools/perf/perf-completion.sh                   |   6 +-
>  tools/perf/perf.c                               |  28 ++++
>  tools/perf/util/build-id.c                      | 105 ++++++++++--
>  tools/perf/util/build-id.h                      |   4 +
>  tools/perf/util/parse-events.c                  | 210 +++++++++++++++++-------
>  tools/perf/util/parse-events.h                  |  11 +-
>  tools/perf/util/parse-options.c                 |   5 +-
>  tools/perf/util/probe-event.c                   | 117 ++++++++++++-
>  tools/perf/util/sort.c                          |   9 +
>  17 files changed, 542 insertions(+), 127 deletions(-)

Pulled, but let me rant a bit:

recent changes to perf bench broke the build on an Ubuntu image I 
have:

  CC       bench/futex-hash.o
In file included from bench/futex-hash.c:17:0:
bench/futex.h:73:19: error: conflicting types for 
‘pthread_attr_setaffinity_np’
 static inline int pthread_attr_setaffinity_np(pthread_attr_t *attr,
                   ^
In file included from bench/futex.h:72:0,
                 from bench/futex-hash.c:17:
/usr/include/pthread.h:407:12: note: previous declaration of 
‘pthread_attr_setaffinity_np’ was here
 extern int pthread_attr_setaffinity_np (pthread_attr_t *__attr,
            ^

So this is a config detection system confusion, and while trying to 
read the code I got lost in it. For example in:

  0695e57b9a6a perf tools: Factor features display code

Firstly, 'factor' isn't a verb we use for code, 'factor out' is. But 
it's not really factoring out - it separates the code. Did this title 
want to say:

   perf tools: Separate feature display code into three parts

? The title was absolutely unreadable.

Then it introduces two new makefile variables: LIB_FEATURE_TESTS and 
VF_FEATURE_TESTS. LIB_FEATURE_TESTS is reasonably self-explanatory, 
but wth is VF_FEATURE_TESTS?

More importantly, why isn't there a _single_ comment explaining their 
use in the Makefile - _especially_ since CORE_FEATURE_TESTS is 
explained so well, it's not like a bad example had to be followed.

Using cryptic abbreviations like 'VF' adds insult to injury. It took 
me some time to figure out that it probably means 'Verbose Features'.

New feature detection adds to all these variables so people will just 
guess to which to add and why.

Guys, this particular change was rushed and not explained well enough, 
and it made debugging from that point on harder. Please slow down a 
bit.

</rant>

Back to the bug, using the (now decyphered VF=1), the pthread-attr 
test fails:

...   pthread-attr-setaffinity-np: [ OFF ]

but that failure is bogus - this is why the build bug triggers. The 
failure happens because:

triton:~/tip/tools/perf/config/feature-checks> make 
test-pthread-attr-setaffinity-np.bin
gcc -MD  -o test-pthread-attr-setaffinity-np.bin 
test-pthread-attr-setaffinity-np.c  -D_GNU_SOURCE -Werror -lpthread
test-pthread-attr-setaffinity-np.c: In function ‘main’:
test-pthread-attr-setaffinity-np.c:11:2: error: null argument where non-null required (argument 3) [-Werror=nonnull]
  ret = pthread_attr_setaffinity_np(&thread_attr, 0, NULL);
  ^
cc1: all warnings being treated as errors
Makefile:53: recipe for target 'test-pthread-attr-setaffinity-np.bin' failed

It's a new -Werror=nonnull warning by GCC 4.9.1 that appears to have 
caused this - extending the testcase to a more substantial 
pthread_attr_setaffinity_np() use fixes it for me and perf builds 
fine.

Find further below another feature detection message fix for the same 
system - libslang-dev was replaced by libslang2-dev.

Thanks,

	Ingo

Signed-off-by: Ingo Molnar <mingo@kernel.org>

diff --git a/tools/perf/config/feature-checks/test-pthread-attr-setaffinity-np.c b/tools/perf/config/feature-checks/test-pthread-attr-setaffinity-np.c
index 0a0d3ecb4e8a..85ab83e6a42f 100644
--- a/tools/perf/config/feature-checks/test-pthread-attr-setaffinity-np.c
+++ b/tools/perf/config/feature-checks/test-pthread-attr-setaffinity-np.c
@@ -5,10 +5,12 @@ int main(void)
 {
 	int ret = 0;
 	pthread_attr_t thread_attr;
+	cpu_set_t cpu_mask;
 
 	pthread_attr_init(&thread_attr);
-	/* don't care abt exact args, just the API itself in libpthread */
-	ret = pthread_attr_setaffinity_np(&thread_attr, 0, NULL);
+	CPU_ZERO(&cpu_mask);
+
+	ret = pthread_attr_setaffinity_np(&thread_attr, sizeof(cpu_mask), &cpu_mask);
 
 	return ret;
 }

Signed-off-by: Ingo Molnar <mingo@kernel.org>

diff --git a/tools/perf/config/Makefile b/tools/perf/config/Makefile
index c3570b5..c13184d 100644
--- a/tools/perf/config/Makefile
+++ b/tools/perf/config/Makefile
@@ -489,7 +489,7 @@ endif
 
 ifndef NO_SLANG
   ifneq ($(feature-libslang), 1)
-    msg := $(warning slang not found, disables TUI support. Please install slang-devel or libslang-dev);
+    msg := $(warning slang not found, disables TUI support. Please install slang-devel or libslang-dev/libslang2-dev);
     NO_SLANG := 1
   else
     # Fedora has /usr/include/slang/slang.h, but ubuntu /usr/include/slang.h