Skip to contentSkip to conversation

Resubmitted forgotten patches

PATCH Applied David Carrillo-Cisneros 4 participants 11 messages Jul 19, 2017 – Jul 26, 2017 perf events

lore
Share & tools
lore
#0 lore raw

cover Resubmitted forgotten patches

Pickup some small fixes that have been forgotten.

David Carrillo-Cisneros (3):
perf tool cgroup: Initialize cgroup refcnt with refcount_set perf tools: Add EXCLUDE_EXTLIBS and EXTRA_PERFLIBS to makefile perf annotate: Process tracing data in pipe mode

Sudeep Holla (1):
tools: perf: Fix linker error when libelf config is disabled

 tools/perf/Makefile.perf      | 10 ++++++++--
 tools/perf/builtin-annotate.c |  1 +
 tools/perf/util/cgroup.c      |  5 ++++-
 3 files changed, 13 insertions(+), 3 deletions(-)
Signature · 1 line
2.13.2.932.g7449e964c-goog
#1 lore raw

PATCH 1/4 perf tool cgroup: Initialize cgroup refcnt with refcount_set

Atomic reference counters were replaced by refcount_t in
commit 79c5fe6db8c7 ("perf/core: Fix error handling in perf_event_alloc()")

In util/cgroup.c atomic_inc was replaced by refcount_inc, but the latter is not mean to initiliaze refcounts with zero value. Add a path to initialize cgrp->refcnt == 0 using refcount_set.

Before this patch:

  $ perf stat -e cycles -C 0 -G /
  perf_before: /usr/local/.../tools/include/linux/refcount.h:108: refcount_inc: Assertion `!(!refcount_inc_not_zero(r))' failed.
  Aborted (core dumped)

After this patch:

  $ perf stat -e cycles -C 0 -G /
  Performance counter stats for 'CPU(s) 0':
     17,516,664      cycles                    /
Change-Id I8f00f61aaecce876e7df448bd7f850b20db13ef1
---
 tools/perf/util/cgroup.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)
tools/perf/util/cgroup.c +4 -1
index 03347748f3fa..7bbc19b3caf3 100644
@@ -133,7 +133,10 @@ static int add_cgroup(struct perf_evlist *evlist, char *str)
 
 	return -1;
 found:
-	refcount_inc(&cgrp->refcnt);
+	if (refcount_read(&cgrp->refcnt) == 0)
+		refcount_set(&cgrp->refcnt, 1);
+	else
+		refcount_inc(&cgrp->refcnt);
 	counter->cgrp = cgrp;
 	return 0;
 }
Signature · 1 line
2.13.2.932.g7449e964c-goog
maintainer Acked-by Elena Reshetova on v1 1/4
#2 lore raw

Re: [PATCH 1/4] perf tool cgroup: Initialize cgroup refcnt with refcount_set

Em Tue, Jul 18, 2017 at 06:18:36PM -0700, David Carrillo-Cisneros escreveu:

Quoted 13 lines

Atomic reference counters were replaced by refcount_t in
commit 79c5fe6db8c7 ("perf/core: Fix error handling in perf_event_alloc()")

In util/cgroup.c atomic_inc was replaced by refcount_inc, but the latter is not mean to initiliaze refcounts with zero value. Add a path to initialize cgrp->refcnt == 0 using refcount_set.

Before this patch:

  $ perf stat -e cycles -C 0 -G /
  perf_before: /usr/local/.../tools/include/linux/refcount.h:108: refcount_inc: Assertion `!(!refcount_inc_not_zero(r))' failed.
  Aborted (core dumped)
Quoted 5 lines

After this patch:

  $ perf stat -e cycles -C 0 -G /
  Performance counter stats for 'CPU(s) 0':
     17,516,664      cycles                    /

Ok, so this one was also reported by Brendan and I came up with an alternative patch, that follows the usual sequence of steps, see below.

- Arnaldo

commit b13ca9843c4ea07c5a1dbf0295986b5dfeb6ef6f

Author: Arnaldo Carvalho de Melo <[email protected]>
Date:   Tue Jul 18 20:20:19 2017 -0300

perf cgroup: Fix refcount usage

When converting from atomic_t to refcount_t we didn't follow the usual step of initializing it to one before taking any new reference, which trips over checking if taking a reference for a freed refcount_t, fix it.

Brendan's report:

---
It's 4.12-rc7, with node v4.4.1. I'm building 4.13-rc1 now, as I hit what I think is another unrelated perf bug and I'm starting to wonder what else is broken on that version:

(root) /mnt/src/linux-4.12-rc7/tools/perf # ./perf record -F 99 -a -e cpu-clock --cgroup=docker/f9e9d5df065b14646e8a11edc837a13877fd90c171137b2ba3feb67a0201cb65 -g
perf: /mnt/src/linux-4.12-rc7/tools/include/linux/refcount.h:108:
refcount_inc: Assertion `!(!refcount_inc_not_zero(r))' failed.
Aborted

that used to work...
---

Testing it:

Before:

# perf stat -e cycles -C 0 --cgroup /
perf: /home/acme/git/linux/tools/include/linux/refcount.h:108: refcount_inc: Assertion `!(!refcount_inc_not_zero(r))' failed. Aborted (core dumped)
#

After:

# perf stat -e cycles -C 0 --cgroup /
^C
Performance counter stats for 'CPU(s) 0':

132,081,393 cycles /

2.492942763 seconds time elapsed

#

Cc Alexander Shishkin <[email protected]>
Cc David Ahern <[email protected]>
Cc David Carrillo-Cisneros <[email protected]>
Cc Kees Kook <[email protected]>
Cc Krister Johansen <[email protected]>
Cc Paul Turner <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
Cc Sudeep Holla <[email protected]>
Cc Thomas-Mich Richter <[email protected]>
Cc Wang Nan <[email protected]>
Fixes 79c5fe6db8c7 ("perf cgroup: Convert cgroup_sel.refcnt from atomic_t to refcount_t")
tools/perf/util/cgroup.c +5 -3
index 03347748f3fa..0e77bc9e5f3c 100644
@@ -98,8 +98,10 @@ static int add_cgroup(struct perf_evlist *evlist, char *str)
 		cgrp = counter->cgrp;
 		if (!cgrp)
 			continue;
-		if (!strcmp(cgrp->name, str))
+		if (!strcmp(cgrp->name, str)) {
+			refcount_inc(&cgrp->refcnt);
 			break;
+		}
 
 		cgrp = NULL;
 	}
@@ -110,6 +112,7 @@ static int add_cgroup(struct perf_evlist *evlist, char *str)
 			return -1;
 
 		cgrp->name = str;
+		refcount_set(&cgrp->refcnt, 1);
 
 		cgrp->fd = open_cgroup(str);
 		if (cgrp->fd == -1) {
@@ -128,12 +131,11 @@ static int add_cgroup(struct perf_evlist *evlist, char *str)
 			goto found;
 		n++;
 	}
-	if (refcount_read(&cgrp->refcnt) == 0)
+	if (refcount_dec_and_test(&cgrp->refcnt))
 		free(cgrp);
 
 	return -1;
 found:
-	refcount_inc(&cgrp->refcnt);
 	counter->cgrp = cgrp;
 	return 0;
 }
#3 lore raw

PATCH 2/4 perf tools: Add EXCLUDE_EXTLIBS and EXTRA_PERFLIBS to makefile

In reply to David Carrillo-Cisneros

The goal is to allow users to override linking of libraries that were automatically added to PERFLIBS.

EXCLUDE_EXTLIBS contains linker flags to be removed from LIBS while EXTRA_PERFLIBS contains linker flags to be added.

My use case is to force certain library to be build statically, e.g. for libelf:

EXCLUDE_EXTLIBS=-lelf EXTRA_PERFLIBS=path/libelf.a

---
 tools/perf/Makefile.perf | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
tools/perf/Makefile.perf +7 -1
index 5008f51a08a2..100a6c1670c8 100644
@@ -33,6 +33,11 @@ include ../scripts/utilities.mak
 #
 # Define EXTRA_CFLAGS=-m64 or EXTRA_CFLAGS=-m32 as appropriate for cross-builds.
 #
+# Define EXCLUDE_EXTLIBS=-lmylib to exclude libmylib from the auto-generated
+# EXTLIBS.
+#
+# Define EXTRA_PERFLIBS to pass extra libraries to PERFLIBS.
+#
 # Define NO_DWARF if you do not want debug-info analysis feature at all.
 #
 # Define WERROR=0 to disable treating any warnings as errors.
@@ -352,7 +357,8 @@ ifdef ASCIIDOC8
   export ASCIIDOC8
 endif
 
-LIBS = -Wl,--whole-archive $(PERFLIBS) -Wl,--no-whole-archive -Wl,--start-group $(EXTLIBS) -Wl,--end-group
+EXTLIBS := $(call filter-out,$(EXCLUDE_EXTLIBS),$(EXTLIBS))
+LIBS = -Wl,--whole-archive $(PERFLIBS) $(EXTRA_PERFLIBS) -Wl,--no-whole-archive -Wl,--start-group $(EXTLIBS) -Wl,--end-group
 
 ifeq ($(USE_CLANG), 1)
   CLANGLIBS_LIST = AST Basic CodeGen Driver Frontend Lex Tooling Edit Sema Analysis Parse Serialization
Signature · 1 line
2.13.2.932.g7449e964c-goog
#4 lore raw

[tip:perf/core] perf tools: Add EXCLUDE_EXTLIBS and EXTRA_PERFLIBS to makefile

perf tools: Add EXCLUDE_EXTLIBS and EXTRA_PERFLIBS to makefile
Commit-ID:  cb281fea4b0a326d2a2104f8ffae2b6895c561fd
Gitweb:     http://git.kernel.org/tip/cb281fea4b0a326d2a2104f8ffae2b6895c561fd
Author:     David Carrillo-Cisneros <[email protected]>
AuthorDate: Tue, 18 Jul 2017 18:18:37 -0700
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 25 Jul 2017 11:23:51 -0300

perf tools: Add EXCLUDE_EXTLIBS and EXTRA_PERFLIBS to makefile

The goal is to allow users to override linking of libraries that were automatically added to PERFLIBS.

EXCLUDE_EXTLIBS contains linker flags to be removed from LIBS while EXTRA_PERFLIBS contains linker flags to be added.

My use case is to force certain library to be build statically, e.g. for libelf:

EXCLUDE_EXTLIBS=-lelf EXTRA_PERFLIBS=path/libelf.a

---
 tools/perf/Makefile.perf | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
tools/perf/Makefile.perf +7 -1
index 5008f51..100a6c1 100644
@@ -33,6 +33,11 @@ include ../scripts/utilities.mak
 #
 # Define EXTRA_CFLAGS=-m64 or EXTRA_CFLAGS=-m32 as appropriate for cross-builds.
 #
+# Define EXCLUDE_EXTLIBS=-lmylib to exclude libmylib from the auto-generated
+# EXTLIBS.
+#
+# Define EXTRA_PERFLIBS to pass extra libraries to PERFLIBS.
+#
 # Define NO_DWARF if you do not want debug-info analysis feature at all.
 #
 # Define WERROR=0 to disable treating any warnings as errors.
@@ -352,7 +357,8 @@ ifdef ASCIIDOC8
   export ASCIIDOC8
 endif
 
-LIBS = -Wl,--whole-archive $(PERFLIBS) -Wl,--no-whole-archive -Wl,--start-group $(EXTLIBS) -Wl,--end-group
+EXTLIBS := $(call filter-out,$(EXCLUDE_EXTLIBS),$(EXTLIBS))
+LIBS = -Wl,--whole-archive $(PERFLIBS) $(EXTRA_PERFLIBS) -Wl,--no-whole-archive -Wl,--start-group $(EXTLIBS) -Wl,--end-group
 
 ifeq ($(USE_CLANG), 1)
   CLANGLIBS_LIST = AST Basic CodeGen Driver Frontend Lex Tooling Edit Sema Analysis Parse Serialization
#5 lore raw

PATCH 3/4 perf annotate: Process tracing data in pipe mode

In reply to David Carrillo-Cisneros

perf annotate was missing the handler for tracing data records.

Prior to this patch we obtained "unhandled" records when piping trace events to perf annotate (using -D option to show the dump_printf messages in process_event_synth_tracing_data_stub):

  $ perf record -o - -e block:bio_free sleep 2 | perf annotate -D --stdio
  ...
  0x78 [0xc]: PERF_RECORD_TRACING_DATA: unhandled!
  ...
---
 tools/perf/builtin-annotate.c | 1 +
 1 file changed, 1 insertion(+)
tools/perf/builtin-annotate.c +1 -0
index 5205408e795b..ffe28002dc4f 100644
@@ -397,6 +397,7 @@ int cmd_annotate(int argc, const char **argv)
 			.namespaces = perf_event__process_namespaces,
 			.attr	= perf_event__process_attr,
 			.build_id = perf_event__process_build_id,
+			.tracing_data   = perf_event__process_tracing_data,
 			.feature	= perf_event__process_feature,
 			.ordered_events = true,
 			.ordering_requires_timestamps = true,
Signature · 1 line
2.13.2.932.g7449e964c-goog
#6 lore raw

[tip:perf/core] perf annotate: Process tracing data in pipe mode

perf annotate: Process tracing data in pipe mode
Commit-ID:  f4849599086c6462d65543637058c9b55f4803e4
Gitweb:     http://git.kernel.org/tip/f4849599086c6462d65543637058c9b55f4803e4
Author:     David Carrillo-Cisneros <[email protected]>
AuthorDate: Tue, 18 Jul 2017 18:18:38 -0700
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 25 Jul 2017 11:23:52 -0300

perf annotate: Process tracing data in pipe mode

'perf annotate' was missing the handler for tracing data records.

Prior to this patch we obtained "unhandled" records when piping trace events to perf annotate (using -D option to show the dump_printf messages in process_event_synth_tracing_data_stub):

  $ perf record -o - -e block:bio_free sleep 2 | perf annotate -D --stdio
  ...
  0x78 [0xc]: PERF_RECORD_TRACING_DATA: unhandled!
  ...
---
 tools/perf/builtin-annotate.c | 1 +
 1 file changed, 1 insertion(+)
tools/perf/builtin-annotate.c +1 -0
index 7e33278..6db782d 100644
@@ -396,6 +396,7 @@ int cmd_annotate(int argc, const char **argv)
 			.namespaces = perf_event__process_namespaces,
 			.attr	= perf_event__process_attr,
 			.build_id = perf_event__process_build_id,
+			.tracing_data   = perf_event__process_tracing_data,
 			.feature	= perf_event__process_feature,
 			.ordered_events = true,
 			.ordering_requires_timestamps = true,
#7 lore raw

PATCH 4/4 tools: perf: Fix linker error when libelf config is disabled

In reply to David Carrillo-Cisneros

From: Sudeep Holla <[email protected]>

When libelf is disabled in the configuration, we get the following linker error:
LINK libperf-jvmti.so
ld: cannot find -lelf
Makefile.perf:515: recipe for target 'libperf-jvmti.so' failed

Jiri pointed out that both librt and libelf are not really required. So this patch fixes the linker error by getting rid of unwanted libraries in the linker stage.

Fixes 209045adc2bb ("perf tools: add JVMTI agent library")
Cc Peter Zijlstra <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Stephane Eranian <[email protected]>
---
 tools/perf/Makefile.perf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/Makefile.perf +1 -1
index 100a6c1670c8..d66f90e6be5c 100644
@@ -518,7 +518,7 @@ $(LIBJVMTI_IN): FORCE
 	$(Q)$(MAKE) -f $(srctree)/tools/build/Makefile.build dir=jvmti obj=jvmti
 
 $(OUTPUT)$(LIBJVMTI): $(LIBJVMTI_IN)
-	$(QUIET_LINK)$(CC) -shared -Wl,-soname -Wl,$(LIBJVMTI) -o $@ $< -lelf -lrt
+	$(QUIET_LINK)$(CC) -shared -Wl,-soname -Wl,$(LIBJVMTI) -o $@ $<
 endif
 
 $(patsubst perf-%,%.o,$(PROGRAMS)): $(wildcard */*.h)
Signature · 1 line
2.13.2.932.g7449e964c-goog
#8 lore raw

[tip:perf/core] perf jvmti: Fix linker error when libelf config is disabled

perf jvmti: Fix linker error when libelf config is disabled
Commit-ID:  5d90faf45427dd76fadbe7a4dc4fce3b6f87b550
Gitweb:     http://git.kernel.org/tip/5d90faf45427dd76fadbe7a4dc4fce3b6f87b550
Author:     Sudeep Holla <[email protected]>
AuthorDate: Tue, 18 Jul 2017 18:18:39 -0700
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 25 Jul 2017 11:23:53 -0300

perf jvmti: Fix linker error when libelf config is disabled

When libelf is disabled in the configuration, we get the following linker error:

LINK libperf-jvmti.so
ld: cannot find -lelf
Makefile.perf:515: recipe for target 'libperf-jvmti.so' failed

Jiri pointed out that both librt and libelf are not really required. So this patch fixes the linker error by getting rid of unwanted libraries in the linker stage.

Cc Alexander Shishkin <[email protected]>
Cc Elena Reshetova <[email protected]>
Cc Kees Kook <[email protected]>
Cc Paul Turner <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Stephane Eranian <[email protected]>
Cc Sudeep Holla <[email protected]>
Cc Wang Nan <[email protected]>
Fixes 209045adc2bb ("perf tools: add JVMTI agent library")
---
 tools/perf/Makefile.perf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
tools/perf/Makefile.perf +1 -1
index 100a6c1..d66f90e 100644
@@ -518,7 +518,7 @@ $(LIBJVMTI_IN): FORCE
 	$(Q)$(MAKE) -f $(srctree)/tools/build/Makefile.build dir=jvmti obj=jvmti
 
 $(OUTPUT)$(LIBJVMTI): $(LIBJVMTI_IN)
-	$(QUIET_LINK)$(CC) -shared -Wl,-soname -Wl,$(LIBJVMTI) -o $@ $< -lelf -lrt
+	$(QUIET_LINK)$(CC) -shared -Wl,-soname -Wl,$(LIBJVMTI) -o $@ $<
 endif
 
 $(patsubst perf-%,%.o,$(PROGRAMS)): $(wildcard */*.h)
Acked-by on the v1 series
#9 lore raw
In reply to David Carrillo-Cisneros
Quoted 9 lines · David Carrillo-Cisneros

Pickup some small fixes that have been forgotten.

David Carrillo-Cisneros (3):
perf tool cgroup: Initialize cgroup refcnt with refcount_set perf tools: Add EXCLUDE_EXTLIBS and EXTRA_PERFLIBS to makefile perf annotate: Process tracing data in pipe mode

Sudeep Holla (1):
tools: perf: Fix linker error when libelf config is disabled

for patchset:

thanks,
jirka