[PATCH v2 0/2] perf sched time -g/--call-graph option & perf

[PATCH v2 0/2] perf sched time -g/--call-graph option & perf

[Yang Jihong]

Changes since v1:
 - patch1: Add Reviewed-by tag from Ian and fixes-tag
 - patch2: Add Reviewed-by tag from Ian

Yang Jihong (2):
  perf sched timehist: Fix -g/--call-graph option failure
  perf evsel: Use evsel__name_is() helper

 tools/perf/builtin-kmem.c               |  2 +-
 tools/perf/builtin-sched.c              | 11 ++++---
 tools/perf/builtin-script.c             |  2 +-
 tools/perf/builtin-trace.c              |  4 +--
 tools/perf/tests/evsel-roundtrip-name.c |  4 +--
 tools/perf/tests/parse-events.c         | 39 +++++++++----------------
 6 files changed, 27 insertions(+), 35 deletions(-)

-- 
2.25.1

[PATCH v2 1/2] perf sched timehist: Fix -g/--call-graph option failure

[Yang Jihong]

When perf-sched enables the call-graph recording, sample_type of dummy
event does not have PERF_SAMPLE_CALLCHAIN, timehist_check_attr() checks
that the evsel does not have a callchain, and set show_callchain to 0.

Currently perf sched timehist only saves callchain when processing
sched:sched_switch event, timehist_check_attr() only needs to determine
whether the event has PERF_SAMPLE_CALLCHAIN.

Before:
  # perf sched record -g true
  [ perf record: Woken up 0 times to write data ]
  [ perf record: Captured and wrote 4.153 MB perf.data (7536 samples) ]
  # perf sched timehist
  Samples do not have callchains.
             time    cpu  task name                       wait time  sch delay   run time
                          [tid/pid]                          (msec)     (msec)     (msec)
  --------------- ------  ------------------------------  ---------  ---------  ---------
    147851.826019 [0000]  perf[285035]                        0.000      0.000      0.000
    147851.826029 [0000]  migration/0[15]                     0.000      0.003      0.009
    147851.826063 [0001]  perf[285035]                        0.000      0.000      0.000
    147851.826069 [0001]  migration/1[21]                     0.000      0.003      0.006
  <SNIP>

After:
  # perf sched record -g true
  [ perf record: Woken up 1 times to write data ]
  [ perf record: Captured and wrote 2.572 MB perf.data (822 samples) ]
  # perf sched timehist
             time    cpu  task name                       wait time  sch delay   run time
                          [tid/pid]                          (msec)     (msec)     (msec)
  --------------- ------  ------------------------------  ---------  ---------  ---------
    144193.035164 [0000]  perf[277062]                        0.000      0.000      0.000    __traceiter_sched_switch <- __traceiter_sched_switch <- __sched_text_start <- preempt_schedule_common <- __cond_resched <- __wait_for_common <- wait_for_completion
    144193.035174 [0000]  migration/0[15]                     0.000      0.003      0.009    __traceiter_sched_switch <- __traceiter_sched_switch <- __sched_text_start <- smpboot_thread_fn <- kthread <- ret_from_fork
    144193.035207 [0001]  perf[277062]                        0.000      0.000      0.000    __traceiter_sched_switch <- __traceiter_sched_switch <- __sched_text_start <- preempt_schedule_common <- __cond_resched <- __wait_for_common <- wait_for_completion
    144193.035214 [0001]  migration/1[21]                     0.000      0.003      0.007    __traceiter_sched_switch <- __traceiter_sched_switch <- __sched_text_start <- smpboot_thread_fn <- kthread <- ret_from_fork
  <SNIP>

Fixes: 9c95e4ef0657 ("perf evlist: Add evlist__findnew_tracking_event() helper")
Signed-off-by: Yang Jihong <yangjihong@bytedance.com>
Reviewed-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-sched.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index b248c433529a..1bfb22347371 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -2963,8 +2963,11 @@ static int timehist_check_attr(struct perf_sched *sched,
 			return -1;
 		}
 
-		if (sched->show_callchain && !evsel__has_callchain(evsel)) {
-			pr_info("Samples do not have callchains.\n");
+		/* only need to save callchain related to sched_switch event */
+		if (sched->show_callchain &&
+		    evsel__name_is(evsel, "sched:sched_switch") &&
+		    !evsel__has_callchain(evsel)) {
+			pr_info("Samples of sched_switch event do not have callchains.\n");
 			sched->show_callchain = 0;
 			symbol_conf.use_callchain = 0;
 		}
-- 
2.25.1

[PATCH v2 2/2] perf evsel: Use evsel__name_is() helper

[Yang Jihong]

Code cleanup, replace strcmp(evsel__name(evsel, {NAME})) with
evsel__name_is() helper.

No functional change.

Signed-off-by: Yang Jihong <yangjihong@bytedance.com>
Reviewed-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-kmem.c               |  2 +-
 tools/perf/builtin-sched.c              |  4 +--
 tools/perf/builtin-script.c             |  2 +-
 tools/perf/builtin-trace.c              |  4 +--
 tools/perf/tests/evsel-roundtrip-name.c |  4 +--
 tools/perf/tests/parse-events.c         | 39 +++++++++----------------
 6 files changed, 22 insertions(+), 33 deletions(-)

diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index 9714327fd0ea..6fd95be5032b 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -1408,7 +1408,7 @@ static int __cmd_kmem(struct perf_session *session)
 	}
 
 	evlist__for_each_entry(session->evlist, evsel) {
-		if (!strcmp(evsel__name(evsel), "kmem:mm_page_alloc") &&
+		if (evsel__name_is(evsel, "kmem:mm_page_alloc") &&
 		    evsel__field(evsel, "pfn")) {
 			use_pfn = true;
 			break;
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index 1bfb22347371..0fce7d8986c0 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -2148,7 +2148,7 @@ static bool is_idle_sample(struct perf_sample *sample,
 			   struct evsel *evsel)
 {
 	/* pid 0 == swapper == idle task */
-	if (strcmp(evsel__name(evsel), "sched:sched_switch") == 0)
+	if (evsel__name_is(evsel, "sched:sched_switch"))
 		return evsel__intval(evsel, sample, "prev_pid") == 0;
 
 	return sample->pid == 0;
@@ -2375,7 +2375,7 @@ static bool timehist_skip_sample(struct perf_sched *sched,
 	}
 
 	if (sched->idle_hist) {
-		if (strcmp(evsel__name(evsel), "sched:sched_switch"))
+		if (!evsel__name_is(evsel, "sched:sched_switch"))
 			rc = true;
 		else if (evsel__intval(evsel, sample, "prev_pid") != 0 &&
 			 evsel__intval(evsel, sample, "next_pid") != 0)
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 37088cc0ff1b..cc981531ec00 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -3471,7 +3471,7 @@ static int check_ev_match(char *dir_name, char *scriptname,
 
 			match = 0;
 			evlist__for_each_entry(session->evlist, pos) {
-				if (!strcmp(evsel__name(pos), evname)) {
+				if (evsel__name_is(pos, evname)) {
 					match = 1;
 					break;
 				}
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 90eaff8c0f6e..9b93807a1906 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -4902,7 +4902,7 @@ int cmd_trace(int argc, const char **argv)
 		goto out;
 	}
 	trace.syscalls.events.bpf_output = evlist__last(trace.evlist);
-	assert(!strcmp(evsel__name(trace.syscalls.events.bpf_output), "__augmented_syscalls__"));
+	assert(evsel__name_is(trace.syscalls.events.bpf_output), "__augmented_syscalls__");
 skip_augmentation:
 #endif
 	err = -1;
@@ -4959,7 +4959,7 @@ int cmd_trace(int argc, const char **argv)
 	 */
 	if (trace.syscalls.events.bpf_output) {
 		evlist__for_each_entry(trace.evlist, evsel) {
-			bool raw_syscalls_sys_exit = strcmp(evsel__name(evsel), "raw_syscalls:sys_exit") == 0;
+			bool raw_syscalls_sys_exit = evsel__name_is(evsel, "raw_syscalls:sys_exit");
 
 			if (raw_syscalls_sys_exit) {
 				trace.raw_augmented_syscalls = true;
diff --git a/tools/perf/tests/evsel-roundtrip-name.c b/tools/perf/tests/evsel-roundtrip-name.c
index 15ff86f9da0b..1922cac13a24 100644
--- a/tools/perf/tests/evsel-roundtrip-name.c
+++ b/tools/perf/tests/evsel-roundtrip-name.c
@@ -37,7 +37,7 @@ static int perf_evsel__roundtrip_cache_name_test(void)
 					continue;
 				}
 				evlist__for_each_entry(evlist, evsel) {
-					if (strcmp(evsel__name(evsel), name)) {
+					if (!evsel__name_is(evsel, name)) {
 						pr_debug("%s != %s\n", evsel__name(evsel), name);
 						ret = TEST_FAIL;
 					}
@@ -71,7 +71,7 @@ static int perf_evsel__name_array_test(const char *const names[], int nr_names)
 			continue;
 		}
 		evlist__for_each_entry(evlist, evsel) {
-			if (strcmp(evsel__name(evsel), names[i])) {
+			if (!evsel__name_is(evsel, names[i])) {
 				pr_debug("%s != %s\n", evsel__name(evsel), names[i]);
 				ret = TEST_FAIL;
 			}
diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index feb5727584d1..0b70451451b3 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -470,8 +470,7 @@ static int test__checkevent_breakpoint_modifier(struct evlist *evlist)
 	TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel);
 	TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
 	TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip);
-	TEST_ASSERT_VAL("wrong name",
-			!strcmp(evsel__name(evsel), "mem:0:u"));
+	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "mem:0:u"));
 
 	return test__checkevent_breakpoint(evlist);
 }
@@ -484,8 +483,7 @@ static int test__checkevent_breakpoint_x_modifier(struct evlist *evlist)
 	TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->core.attr.exclude_kernel);
 	TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
 	TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip);
-	TEST_ASSERT_VAL("wrong name",
-			!strcmp(evsel__name(evsel), "mem:0:x:k"));
+	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "mem:0:x:k"));
 
 	return test__checkevent_breakpoint_x(evlist);
 }
@@ -498,8 +496,7 @@ static int test__checkevent_breakpoint_r_modifier(struct evlist *evlist)
 	TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel);
 	TEST_ASSERT_VAL("wrong exclude_hv", !evsel->core.attr.exclude_hv);
 	TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip);
-	TEST_ASSERT_VAL("wrong name",
-			!strcmp(evsel__name(evsel), "mem:0:r:hp"));
+	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "mem:0:r:hp"));
 
 	return test__checkevent_breakpoint_r(evlist);
 }
@@ -512,8 +509,7 @@ static int test__checkevent_breakpoint_w_modifier(struct evlist *evlist)
 	TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel);
 	TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
 	TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip);
-	TEST_ASSERT_VAL("wrong name",
-			!strcmp(evsel__name(evsel), "mem:0:w:up"));
+	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "mem:0:w:up"));
 
 	return test__checkevent_breakpoint_w(evlist);
 }
@@ -526,8 +522,7 @@ static int test__checkevent_breakpoint_rw_modifier(struct evlist *evlist)
 	TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->core.attr.exclude_kernel);
 	TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
 	TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip);
-	TEST_ASSERT_VAL("wrong name",
-			!strcmp(evsel__name(evsel), "mem:0:rw:kp"));
+	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "mem:0:rw:kp"));
 
 	return test__checkevent_breakpoint_rw(evlist);
 }
@@ -540,8 +535,7 @@ static int test__checkevent_breakpoint_modifier_name(struct evlist *evlist)
 	TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel);
 	TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
 	TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip);
-	TEST_ASSERT_VAL("wrong name",
-			!strcmp(evsel__name(evsel), "breakpoint"));
+	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint"));
 
 	return test__checkevent_breakpoint(evlist);
 }
@@ -554,8 +548,7 @@ static int test__checkevent_breakpoint_x_modifier_name(struct evlist *evlist)
 	TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->core.attr.exclude_kernel);
 	TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
 	TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip);
-	TEST_ASSERT_VAL("wrong name",
-			!strcmp(evsel__name(evsel), "breakpoint"));
+	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint"));
 
 	return test__checkevent_breakpoint_x(evlist);
 }
@@ -568,8 +561,7 @@ static int test__checkevent_breakpoint_r_modifier_name(struct evlist *evlist)
 	TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel);
 	TEST_ASSERT_VAL("wrong exclude_hv", !evsel->core.attr.exclude_hv);
 	TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip);
-	TEST_ASSERT_VAL("wrong name",
-			!strcmp(evsel__name(evsel), "breakpoint"));
+	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint"));
 
 	return test__checkevent_breakpoint_r(evlist);
 }
@@ -582,8 +574,7 @@ static int test__checkevent_breakpoint_w_modifier_name(struct evlist *evlist)
 	TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel);
 	TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
 	TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip);
-	TEST_ASSERT_VAL("wrong name",
-			!strcmp(evsel__name(evsel), "breakpoint"));
+	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint"));
 
 	return test__checkevent_breakpoint_w(evlist);
 }
@@ -596,8 +587,7 @@ static int test__checkevent_breakpoint_rw_modifier_name(struct evlist *evlist)
 	TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->core.attr.exclude_kernel);
 	TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
 	TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip);
-	TEST_ASSERT_VAL("wrong name",
-			!strcmp(evsel__name(evsel), "breakpoint"));
+	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint"));
 
 	return test__checkevent_breakpoint_rw(evlist);
 }
@@ -609,12 +599,12 @@ static int test__checkevent_breakpoint_2_events(struct evlist *evlist)
 	TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries);
 
 	TEST_ASSERT_VAL("wrong type", PERF_TYPE_BREAKPOINT == evsel->core.attr.type);
-	TEST_ASSERT_VAL("wrong name", !strcmp(evsel__name(evsel), "breakpoint1"));
+	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint1"));
 
 	evsel = evsel__next(evsel);
 
 	TEST_ASSERT_VAL("wrong type", PERF_TYPE_BREAKPOINT == evsel->core.attr.type);
-	TEST_ASSERT_VAL("wrong name", !strcmp(evsel__name(evsel), "breakpoint2"));
+	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint2"));
 
 	return TEST_OK;
 }
@@ -691,15 +681,14 @@ static int test__checkevent_pmu_name(struct evlist *evlist)
 	TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries);
 	TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->core.attr.type);
 	TEST_ASSERT_VAL("wrong config", test_config(evsel, 1));
-	TEST_ASSERT_VAL("wrong name", !strcmp(evsel__name(evsel), "krava"));
+	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "krava"));
 
 	/* cpu/config=2/u" */
 	evsel = evsel__next(evsel);
 	TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries);
 	TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->core.attr.type);
 	TEST_ASSERT_VAL("wrong config", test_config(evsel, 2));
-	TEST_ASSERT_VAL("wrong name",
-			!strcmp(evsel__name(evsel), "cpu/config=2/u"));
+	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "cpu/config=2/u"));
 
 	return TEST_OK;
 }
-- 
2.25.1

Re: [PATCH v2 2/2] perf evsel: Use evsel__name_is() helper

[Arnaldo Carvalho de Melo]

On Mon, Apr 01, 2024 at 02:27:24PM +0800, Yang Jihong wrote:
> Code cleanup, replace strcmp(evsel__name(evsel, {NAME})) with
> evsel__name_is() helper.
> 
> No functional change.

Well, its not building:

builtin-trace.c: In function ‘cmd_trace’:
builtin-trace.c:4919:90: error: macro "assert" passed 2 arguments, but takes just 1
 4919 |         assert(evsel__name_is(trace.syscalls.events.bpf_output), "__augmented_syscalls__");
      |                                                                                          ^
In file included from /home/acme/git/perf-tools-next/tools/include/linux/kernel.h:7,
                 from /home/acme/git/perf-tools-next/tools/include/linux/refcount.h:42,
                 from util/cgroup.h:6,
                 from builtin-trace.c:29:
/usr/include/assert.h:50: note: macro "assert" defined here

I'll fix it, but please test build things next time.

- Arnaldo
 
> Signed-off-by: Yang Jihong <yangjihong@bytedance.com>
> Reviewed-by: Ian Rogers <irogers@google.com>
> ---
>  tools/perf/builtin-kmem.c               |  2 +-
>  tools/perf/builtin-sched.c              |  4 +--
>  tools/perf/builtin-script.c             |  2 +-
>  tools/perf/builtin-trace.c              |  4 +--
>  tools/perf/tests/evsel-roundtrip-name.c |  4 +--
>  tools/perf/tests/parse-events.c         | 39 +++++++++----------------
>  6 files changed, 22 insertions(+), 33 deletions(-)
> 
> diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
> index 9714327fd0ea..6fd95be5032b 100644
> --- a/tools/perf/builtin-kmem.c
> +++ b/tools/perf/builtin-kmem.c
> @@ -1408,7 +1408,7 @@ static int __cmd_kmem(struct perf_session *session)
>  	}
>  
>  	evlist__for_each_entry(session->evlist, evsel) {
> -		if (!strcmp(evsel__name(evsel), "kmem:mm_page_alloc") &&
> +		if (evsel__name_is(evsel, "kmem:mm_page_alloc") &&
>  		    evsel__field(evsel, "pfn")) {
>  			use_pfn = true;
>  			break;
> diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> index 1bfb22347371..0fce7d8986c0 100644
> --- a/tools/perf/builtin-sched.c
> +++ b/tools/perf/builtin-sched.c
> @@ -2148,7 +2148,7 @@ static bool is_idle_sample(struct perf_sample *sample,
>  			   struct evsel *evsel)
>  {
>  	/* pid 0 == swapper == idle task */
> -	if (strcmp(evsel__name(evsel), "sched:sched_switch") == 0)
> +	if (evsel__name_is(evsel, "sched:sched_switch"))
>  		return evsel__intval(evsel, sample, "prev_pid") == 0;
>  
>  	return sample->pid == 0;
> @@ -2375,7 +2375,7 @@ static bool timehist_skip_sample(struct perf_sched *sched,
>  	}
>  
>  	if (sched->idle_hist) {
> -		if (strcmp(evsel__name(evsel), "sched:sched_switch"))
> +		if (!evsel__name_is(evsel, "sched:sched_switch"))
>  			rc = true;
>  		else if (evsel__intval(evsel, sample, "prev_pid") != 0 &&
>  			 evsel__intval(evsel, sample, "next_pid") != 0)
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index 37088cc0ff1b..cc981531ec00 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -3471,7 +3471,7 @@ static int check_ev_match(char *dir_name, char *scriptname,
>  
>  			match = 0;
>  			evlist__for_each_entry(session->evlist, pos) {
> -				if (!strcmp(evsel__name(pos), evname)) {
> +				if (evsel__name_is(pos, evname)) {
>  					match = 1;
>  					break;
>  				}
> diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> index 90eaff8c0f6e..9b93807a1906 100644
> --- a/tools/perf/builtin-trace.c
> +++ b/tools/perf/builtin-trace.c
> @@ -4902,7 +4902,7 @@ int cmd_trace(int argc, const char **argv)
>  		goto out;
>  	}
>  	trace.syscalls.events.bpf_output = evlist__last(trace.evlist);
> -	assert(!strcmp(evsel__name(trace.syscalls.events.bpf_output), "__augmented_syscalls__"));
> +	assert(evsel__name_is(trace.syscalls.events.bpf_output), "__augmented_syscalls__");
>  skip_augmentation:
>  #endif
>  	err = -1;
> @@ -4959,7 +4959,7 @@ int cmd_trace(int argc, const char **argv)
>  	 */
>  	if (trace.syscalls.events.bpf_output) {
>  		evlist__for_each_entry(trace.evlist, evsel) {
> -			bool raw_syscalls_sys_exit = strcmp(evsel__name(evsel), "raw_syscalls:sys_exit") == 0;
> +			bool raw_syscalls_sys_exit = evsel__name_is(evsel, "raw_syscalls:sys_exit");
>  
>  			if (raw_syscalls_sys_exit) {
>  				trace.raw_augmented_syscalls = true;
> diff --git a/tools/perf/tests/evsel-roundtrip-name.c b/tools/perf/tests/evsel-roundtrip-name.c
> index 15ff86f9da0b..1922cac13a24 100644
> --- a/tools/perf/tests/evsel-roundtrip-name.c
> +++ b/tools/perf/tests/evsel-roundtrip-name.c
> @@ -37,7 +37,7 @@ static int perf_evsel__roundtrip_cache_name_test(void)
>  					continue;
>  				}
>  				evlist__for_each_entry(evlist, evsel) {
> -					if (strcmp(evsel__name(evsel), name)) {
> +					if (!evsel__name_is(evsel, name)) {
>  						pr_debug("%s != %s\n", evsel__name(evsel), name);
>  						ret = TEST_FAIL;
>  					}
> @@ -71,7 +71,7 @@ static int perf_evsel__name_array_test(const char *const names[], int nr_names)
>  			continue;
>  		}
>  		evlist__for_each_entry(evlist, evsel) {
> -			if (strcmp(evsel__name(evsel), names[i])) {
> +			if (!evsel__name_is(evsel, names[i])) {
>  				pr_debug("%s != %s\n", evsel__name(evsel), names[i]);
>  				ret = TEST_FAIL;
>  			}
> diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
> index feb5727584d1..0b70451451b3 100644
> --- a/tools/perf/tests/parse-events.c
> +++ b/tools/perf/tests/parse-events.c
> @@ -470,8 +470,7 @@ static int test__checkevent_breakpoint_modifier(struct evlist *evlist)
>  	TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel);
>  	TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
>  	TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip);
> -	TEST_ASSERT_VAL("wrong name",
> -			!strcmp(evsel__name(evsel), "mem:0:u"));
> +	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "mem:0:u"));
>  
>  	return test__checkevent_breakpoint(evlist);
>  }
> @@ -484,8 +483,7 @@ static int test__checkevent_breakpoint_x_modifier(struct evlist *evlist)
>  	TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->core.attr.exclude_kernel);
>  	TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
>  	TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip);
> -	TEST_ASSERT_VAL("wrong name",
> -			!strcmp(evsel__name(evsel), "mem:0:x:k"));
> +	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "mem:0:x:k"));
>  
>  	return test__checkevent_breakpoint_x(evlist);
>  }
> @@ -498,8 +496,7 @@ static int test__checkevent_breakpoint_r_modifier(struct evlist *evlist)
>  	TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel);
>  	TEST_ASSERT_VAL("wrong exclude_hv", !evsel->core.attr.exclude_hv);
>  	TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip);
> -	TEST_ASSERT_VAL("wrong name",
> -			!strcmp(evsel__name(evsel), "mem:0:r:hp"));
> +	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "mem:0:r:hp"));
>  
>  	return test__checkevent_breakpoint_r(evlist);
>  }
> @@ -512,8 +509,7 @@ static int test__checkevent_breakpoint_w_modifier(struct evlist *evlist)
>  	TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel);
>  	TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
>  	TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip);
> -	TEST_ASSERT_VAL("wrong name",
> -			!strcmp(evsel__name(evsel), "mem:0:w:up"));
> +	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "mem:0:w:up"));
>  
>  	return test__checkevent_breakpoint_w(evlist);
>  }
> @@ -526,8 +522,7 @@ static int test__checkevent_breakpoint_rw_modifier(struct evlist *evlist)
>  	TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->core.attr.exclude_kernel);
>  	TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
>  	TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip);
> -	TEST_ASSERT_VAL("wrong name",
> -			!strcmp(evsel__name(evsel), "mem:0:rw:kp"));
> +	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "mem:0:rw:kp"));
>  
>  	return test__checkevent_breakpoint_rw(evlist);
>  }
> @@ -540,8 +535,7 @@ static int test__checkevent_breakpoint_modifier_name(struct evlist *evlist)
>  	TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel);
>  	TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
>  	TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip);
> -	TEST_ASSERT_VAL("wrong name",
> -			!strcmp(evsel__name(evsel), "breakpoint"));
> +	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint"));
>  
>  	return test__checkevent_breakpoint(evlist);
>  }
> @@ -554,8 +548,7 @@ static int test__checkevent_breakpoint_x_modifier_name(struct evlist *evlist)
>  	TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->core.attr.exclude_kernel);
>  	TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
>  	TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip);
> -	TEST_ASSERT_VAL("wrong name",
> -			!strcmp(evsel__name(evsel), "breakpoint"));
> +	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint"));
>  
>  	return test__checkevent_breakpoint_x(evlist);
>  }
> @@ -568,8 +561,7 @@ static int test__checkevent_breakpoint_r_modifier_name(struct evlist *evlist)
>  	TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel);
>  	TEST_ASSERT_VAL("wrong exclude_hv", !evsel->core.attr.exclude_hv);
>  	TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip);
> -	TEST_ASSERT_VAL("wrong name",
> -			!strcmp(evsel__name(evsel), "breakpoint"));
> +	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint"));
>  
>  	return test__checkevent_breakpoint_r(evlist);
>  }
> @@ -582,8 +574,7 @@ static int test__checkevent_breakpoint_w_modifier_name(struct evlist *evlist)
>  	TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel);
>  	TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
>  	TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip);
> -	TEST_ASSERT_VAL("wrong name",
> -			!strcmp(evsel__name(evsel), "breakpoint"));
> +	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint"));
>  
>  	return test__checkevent_breakpoint_w(evlist);
>  }
> @@ -596,8 +587,7 @@ static int test__checkevent_breakpoint_rw_modifier_name(struct evlist *evlist)
>  	TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->core.attr.exclude_kernel);
>  	TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
>  	TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip);
> -	TEST_ASSERT_VAL("wrong name",
> -			!strcmp(evsel__name(evsel), "breakpoint"));
> +	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint"));
>  
>  	return test__checkevent_breakpoint_rw(evlist);
>  }
> @@ -609,12 +599,12 @@ static int test__checkevent_breakpoint_2_events(struct evlist *evlist)
>  	TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries);
>  
>  	TEST_ASSERT_VAL("wrong type", PERF_TYPE_BREAKPOINT == evsel->core.attr.type);
> -	TEST_ASSERT_VAL("wrong name", !strcmp(evsel__name(evsel), "breakpoint1"));
> +	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint1"));
>  
>  	evsel = evsel__next(evsel);
>  
>  	TEST_ASSERT_VAL("wrong type", PERF_TYPE_BREAKPOINT == evsel->core.attr.type);
> -	TEST_ASSERT_VAL("wrong name", !strcmp(evsel__name(evsel), "breakpoint2"));
> +	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint2"));
>  
>  	return TEST_OK;
>  }
> @@ -691,15 +681,14 @@ static int test__checkevent_pmu_name(struct evlist *evlist)
>  	TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries);
>  	TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->core.attr.type);
>  	TEST_ASSERT_VAL("wrong config", test_config(evsel, 1));
> -	TEST_ASSERT_VAL("wrong name", !strcmp(evsel__name(evsel), "krava"));
> +	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "krava"));
>  
>  	/* cpu/config=2/u" */
>  	evsel = evsel__next(evsel);
>  	TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries);
>  	TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->core.attr.type);
>  	TEST_ASSERT_VAL("wrong config", test_config(evsel, 2));
> -	TEST_ASSERT_VAL("wrong name",
> -			!strcmp(evsel__name(evsel), "cpu/config=2/u"));
> +	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "cpu/config=2/u"));
>  
>  	return TEST_OK;
>  }
> -- 
> 2.25.1

Re: [PATCH v2 2/2] perf evsel: Use evsel__name_is() helper

[Arnaldo Carvalho de Melo]

On Tue, Apr 02, 2024 at 06:07:42PM -0300, Arnaldo Carvalho de Melo wrote:
> On Mon, Apr 01, 2024 at 02:27:24PM +0800, Yang Jihong wrote:
> > Code cleanup, replace strcmp(evsel__name(evsel, {NAME})) with
> > evsel__name_is() helper.
> > 
> > No functional change.
> 
> Well, its not building:
> 
> builtin-trace.c: In function ‘cmd_trace’:
> builtin-trace.c:4919:90: error: macro "assert" passed 2 arguments, but takes just 1
>  4919 |         assert(evsel__name_is(trace.syscalls.events.bpf_output), "__augmented_syscalls__");
>       |                                                                                          ^
> In file included from /home/acme/git/perf-tools-next/tools/include/linux/kernel.h:7,
>                  from /home/acme/git/perf-tools-next/tools/include/linux/refcount.h:42,
>                  from util/cgroup.h:6,
>                  from builtin-trace.c:29:
> /usr/include/assert.h:50: note: macro "assert" defined here
> 
> I'll fix it, but please test build things next time.
> 
> - Arnaldo
>  
> > Signed-off-by: Yang Jihong <yangjihong@bytedance.com>
> > Reviewed-by: Ian Rogers <irogers@google.com>
> > ---
> >  tools/perf/builtin-kmem.c               |  2 +-
> >  tools/perf/builtin-sched.c              |  4 +--
> >  tools/perf/builtin-script.c             |  2 +-
> >  tools/perf/builtin-trace.c              |  4 +--
> >  tools/perf/tests/evsel-roundtrip-name.c |  4 +--
> >  tools/perf/tests/parse-events.c         | 39 +++++++++----------------
> >  6 files changed, 22 insertions(+), 33 deletions(-)
> > 
> > diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
> > index 9714327fd0ea..6fd95be5032b 100644
> > --- a/tools/perf/builtin-kmem.c
> > +++ b/tools/perf/builtin-kmem.c
> > @@ -1408,7 +1408,7 @@ static int __cmd_kmem(struct perf_session *session)
> >  	}
> >  
> >  	evlist__for_each_entry(session->evlist, evsel) {
> > -		if (!strcmp(evsel__name(evsel), "kmem:mm_page_alloc") &&
> > +		if (evsel__name_is(evsel, "kmem:mm_page_alloc") &&
> >  		    evsel__field(evsel, "pfn")) {
> >  			use_pfn = true;
> >  			break;
> > diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
> > index 1bfb22347371..0fce7d8986c0 100644
> > --- a/tools/perf/builtin-sched.c
> > +++ b/tools/perf/builtin-sched.c
> > @@ -2148,7 +2148,7 @@ static bool is_idle_sample(struct perf_sample *sample,
> >  			   struct evsel *evsel)
> >  {
> >  	/* pid 0 == swapper == idle task */
> > -	if (strcmp(evsel__name(evsel), "sched:sched_switch") == 0)
> > +	if (evsel__name_is(evsel, "sched:sched_switch"))
> >  		return evsel__intval(evsel, sample, "prev_pid") == 0;
> >  
> >  	return sample->pid == 0;
> > @@ -2375,7 +2375,7 @@ static bool timehist_skip_sample(struct perf_sched *sched,
> >  	}
> >  
> >  	if (sched->idle_hist) {
> > -		if (strcmp(evsel__name(evsel), "sched:sched_switch"))
> > +		if (!evsel__name_is(evsel, "sched:sched_switch"))
> >  			rc = true;
> >  		else if (evsel__intval(evsel, sample, "prev_pid") != 0 &&
> >  			 evsel__intval(evsel, sample, "next_pid") != 0)
> > diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> > index 37088cc0ff1b..cc981531ec00 100644
> > --- a/tools/perf/builtin-script.c
> > +++ b/tools/perf/builtin-script.c
> > @@ -3471,7 +3471,7 @@ static int check_ev_match(char *dir_name, char *scriptname,
> >  
> >  			match = 0;
> >  			evlist__for_each_entry(session->evlist, pos) {
> > -				if (!strcmp(evsel__name(pos), evname)) {
> > +				if (evsel__name_is(pos, evname)) {
> >  					match = 1;
> >  					break;
> >  				}
> > diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> > index 90eaff8c0f6e..9b93807a1906 100644
> > --- a/tools/perf/builtin-trace.c
> > +++ b/tools/perf/builtin-trace.c
> > @@ -4902,7 +4902,7 @@ int cmd_trace(int argc, const char **argv)
> >  		goto out;
> >  	}
> >  	trace.syscalls.events.bpf_output = evlist__last(trace.evlist);
> > -	assert(!strcmp(evsel__name(trace.syscalls.events.bpf_output), "__augmented_syscalls__"));
> > +	assert(evsel__name_is(trace.syscalls.events.bpf_output), "__augmented_syscalls__");


This:

⬢[acme@toolbox perf-tools-next]$ git diff
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index cdb64ed243c416bf..e5fef39c34bf40ff 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -4916,7 +4916,7 @@ int cmd_trace(int argc, const char **argv)
                goto out;
        }
        trace.syscalls.events.bpf_output = evlist__last(trace.evlist);
-       assert(evsel__name_is(trace.syscalls.events.bpf_output), "__augmented_syscalls__");
+       assert(evsel__name_is(trace.syscalls.events.bpf_output, "__augmented_syscalls__"));
 skip_augmentation:
 #endif
        err = -1;
⬢[acme@toolbox perf-tools-next]$

> >  skip_augmentation:
> >  #endif
> >  	err = -1;
> > @@ -4959,7 +4959,7 @@ int cmd_trace(int argc, const char **argv)
> >  	 */
> >  	if (trace.syscalls.events.bpf_output) {
> >  		evlist__for_each_entry(trace.evlist, evsel) {
> > -			bool raw_syscalls_sys_exit = strcmp(evsel__name(evsel), "raw_syscalls:sys_exit") == 0;
> > +			bool raw_syscalls_sys_exit = evsel__name_is(evsel, "raw_syscalls:sys_exit");
> >  
> >  			if (raw_syscalls_sys_exit) {
> >  				trace.raw_augmented_syscalls = true;
> > diff --git a/tools/perf/tests/evsel-roundtrip-name.c b/tools/perf/tests/evsel-roundtrip-name.c
> > index 15ff86f9da0b..1922cac13a24 100644
> > --- a/tools/perf/tests/evsel-roundtrip-name.c
> > +++ b/tools/perf/tests/evsel-roundtrip-name.c
> > @@ -37,7 +37,7 @@ static int perf_evsel__roundtrip_cache_name_test(void)
> >  					continue;
> >  				}
> >  				evlist__for_each_entry(evlist, evsel) {
> > -					if (strcmp(evsel__name(evsel), name)) {
> > +					if (!evsel__name_is(evsel, name)) {
> >  						pr_debug("%s != %s\n", evsel__name(evsel), name);
> >  						ret = TEST_FAIL;
> >  					}
> > @@ -71,7 +71,7 @@ static int perf_evsel__name_array_test(const char *const names[], int nr_names)
> >  			continue;
> >  		}
> >  		evlist__for_each_entry(evlist, evsel) {
> > -			if (strcmp(evsel__name(evsel), names[i])) {
> > +			if (!evsel__name_is(evsel, names[i])) {
> >  				pr_debug("%s != %s\n", evsel__name(evsel), names[i]);
> >  				ret = TEST_FAIL;
> >  			}
> > diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
> > index feb5727584d1..0b70451451b3 100644
> > --- a/tools/perf/tests/parse-events.c
> > +++ b/tools/perf/tests/parse-events.c
> > @@ -470,8 +470,7 @@ static int test__checkevent_breakpoint_modifier(struct evlist *evlist)
> >  	TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel);
> >  	TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
> >  	TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip);
> > -	TEST_ASSERT_VAL("wrong name",
> > -			!strcmp(evsel__name(evsel), "mem:0:u"));
> > +	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "mem:0:u"));
> >  
> >  	return test__checkevent_breakpoint(evlist);
> >  }
> > @@ -484,8 +483,7 @@ static int test__checkevent_breakpoint_x_modifier(struct evlist *evlist)
> >  	TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->core.attr.exclude_kernel);
> >  	TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
> >  	TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip);
> > -	TEST_ASSERT_VAL("wrong name",
> > -			!strcmp(evsel__name(evsel), "mem:0:x:k"));
> > +	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "mem:0:x:k"));
> >  
> >  	return test__checkevent_breakpoint_x(evlist);
> >  }
> > @@ -498,8 +496,7 @@ static int test__checkevent_breakpoint_r_modifier(struct evlist *evlist)
> >  	TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel);
> >  	TEST_ASSERT_VAL("wrong exclude_hv", !evsel->core.attr.exclude_hv);
> >  	TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip);
> > -	TEST_ASSERT_VAL("wrong name",
> > -			!strcmp(evsel__name(evsel), "mem:0:r:hp"));
> > +	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "mem:0:r:hp"));
> >  
> >  	return test__checkevent_breakpoint_r(evlist);
> >  }
> > @@ -512,8 +509,7 @@ static int test__checkevent_breakpoint_w_modifier(struct evlist *evlist)
> >  	TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel);
> >  	TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
> >  	TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip);
> > -	TEST_ASSERT_VAL("wrong name",
> > -			!strcmp(evsel__name(evsel), "mem:0:w:up"));
> > +	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "mem:0:w:up"));
> >  
> >  	return test__checkevent_breakpoint_w(evlist);
> >  }
> > @@ -526,8 +522,7 @@ static int test__checkevent_breakpoint_rw_modifier(struct evlist *evlist)
> >  	TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->core.attr.exclude_kernel);
> >  	TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
> >  	TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip);
> > -	TEST_ASSERT_VAL("wrong name",
> > -			!strcmp(evsel__name(evsel), "mem:0:rw:kp"));
> > +	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "mem:0:rw:kp"));
> >  
> >  	return test__checkevent_breakpoint_rw(evlist);
> >  }
> > @@ -540,8 +535,7 @@ static int test__checkevent_breakpoint_modifier_name(struct evlist *evlist)
> >  	TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel);
> >  	TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
> >  	TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip);
> > -	TEST_ASSERT_VAL("wrong name",
> > -			!strcmp(evsel__name(evsel), "breakpoint"));
> > +	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint"));
> >  
> >  	return test__checkevent_breakpoint(evlist);
> >  }
> > @@ -554,8 +548,7 @@ static int test__checkevent_breakpoint_x_modifier_name(struct evlist *evlist)
> >  	TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->core.attr.exclude_kernel);
> >  	TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
> >  	TEST_ASSERT_VAL("wrong precise_ip", !evsel->core.attr.precise_ip);
> > -	TEST_ASSERT_VAL("wrong name",
> > -			!strcmp(evsel__name(evsel), "breakpoint"));
> > +	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint"));
> >  
> >  	return test__checkevent_breakpoint_x(evlist);
> >  }
> > @@ -568,8 +561,7 @@ static int test__checkevent_breakpoint_r_modifier_name(struct evlist *evlist)
> >  	TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel);
> >  	TEST_ASSERT_VAL("wrong exclude_hv", !evsel->core.attr.exclude_hv);
> >  	TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip);
> > -	TEST_ASSERT_VAL("wrong name",
> > -			!strcmp(evsel__name(evsel), "breakpoint"));
> > +	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint"));
> >  
> >  	return test__checkevent_breakpoint_r(evlist);
> >  }
> > @@ -582,8 +574,7 @@ static int test__checkevent_breakpoint_w_modifier_name(struct evlist *evlist)
> >  	TEST_ASSERT_VAL("wrong exclude_kernel", evsel->core.attr.exclude_kernel);
> >  	TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
> >  	TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip);
> > -	TEST_ASSERT_VAL("wrong name",
> > -			!strcmp(evsel__name(evsel), "breakpoint"));
> > +	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint"));
> >  
> >  	return test__checkevent_breakpoint_w(evlist);
> >  }
> > @@ -596,8 +587,7 @@ static int test__checkevent_breakpoint_rw_modifier_name(struct evlist *evlist)
> >  	TEST_ASSERT_VAL("wrong exclude_kernel", !evsel->core.attr.exclude_kernel);
> >  	TEST_ASSERT_VAL("wrong exclude_hv", evsel->core.attr.exclude_hv);
> >  	TEST_ASSERT_VAL("wrong precise_ip", evsel->core.attr.precise_ip);
> > -	TEST_ASSERT_VAL("wrong name",
> > -			!strcmp(evsel__name(evsel), "breakpoint"));
> > +	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint"));
> >  
> >  	return test__checkevent_breakpoint_rw(evlist);
> >  }
> > @@ -609,12 +599,12 @@ static int test__checkevent_breakpoint_2_events(struct evlist *evlist)
> >  	TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries);
> >  
> >  	TEST_ASSERT_VAL("wrong type", PERF_TYPE_BREAKPOINT == evsel->core.attr.type);
> > -	TEST_ASSERT_VAL("wrong name", !strcmp(evsel__name(evsel), "breakpoint1"));
> > +	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint1"));
> >  
> >  	evsel = evsel__next(evsel);
> >  
> >  	TEST_ASSERT_VAL("wrong type", PERF_TYPE_BREAKPOINT == evsel->core.attr.type);
> > -	TEST_ASSERT_VAL("wrong name", !strcmp(evsel__name(evsel), "breakpoint2"));
> > +	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint2"));
> >  
> >  	return TEST_OK;
> >  }
> > @@ -691,15 +681,14 @@ static int test__checkevent_pmu_name(struct evlist *evlist)
> >  	TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries);
> >  	TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->core.attr.type);
> >  	TEST_ASSERT_VAL("wrong config", test_config(evsel, 1));
> > -	TEST_ASSERT_VAL("wrong name", !strcmp(evsel__name(evsel), "krava"));
> > +	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "krava"));
> >  
> >  	/* cpu/config=2/u" */
> >  	evsel = evsel__next(evsel);
> >  	TEST_ASSERT_VAL("wrong number of entries", 2 == evlist->core.nr_entries);
> >  	TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW == evsel->core.attr.type);
> >  	TEST_ASSERT_VAL("wrong config", test_config(evsel, 2));
> > -	TEST_ASSERT_VAL("wrong name",
> > -			!strcmp(evsel__name(evsel), "cpu/config=2/u"));
> > +	TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "cpu/config=2/u"));
> >  
> >  	return TEST_OK;
> >  }
> > -- 
> > 2.25.1

Re: [External] Re: [PATCH v2 2/2] perf evsel: Use evsel__name_is() helper

[Yang Jihong]

Hello,

On 4/3/24 05:07, Arnaldo Carvalho de Melo wrote:
> On Mon, Apr 01, 2024 at 02:27:24PM +0800, Yang Jihong wrote:
>> Code cleanup, replace strcmp(evsel__name(evsel, {NAME})) with
>> evsel__name_is() helper.
>>
>> No functional change.
> 
> Well, its not building:
> 
> builtin-trace.c: In function ‘cmd_trace’:
> builtin-trace.c:4919:90: error: macro "assert" passed 2 arguments, but takes just 1
>   4919 |         assert(evsel__name_is(trace.syscalls.events.bpf_output), "__augmented_syscalls__");
>        |                                                                                          ^
> In file included from /home/acme/git/perf-tools-next/tools/include/linux/kernel.h:7,
>                   from /home/acme/git/perf-tools-next/tools/include/linux/refcount.h:42,
>                   from util/cgroup.h:6,
>                   from builtin-trace.c:29:
> /usr/include/assert.h:50: note: macro "assert" defined here
> 
> I'll fix it, but please test build things next time.

Sorry. I compiled it in a new environment before. This environment does 
not support bpf skel, and the modified part of the code is included in 
HAVE_BPF_SKEL, so I did not find this problem when I compiled it in my 
environment.


Thanks for the correction. will confirm whether the modified code is 
actually compiled next time.


Thanks,
Yang

Re: [PATCH v2 2/2] perf evsel: Use evsel__name_is() helper

[kernel test robot]

:::::: 
:::::: Manual check reason: "low confidence bisect report"
:::::: 

BCC: lkp@intel.com
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20240401062724.1006010-3-yangjihong@bytedance.com>
References: <20240401062724.1006010-3-yangjihong@bytedance.com>
TO: Yang Jihong <yangjihong@bytedance.com>
TO: peterz@infradead.org
TO: mingo@redhat.com
TO: acme@kernel.org
TO: namhyung@kernel.org
TO: mark.rutland@arm.com
TO: alexander.shishkin@linux.intel.com
TO: jolsa@kernel.org
TO: irogers@google.com
TO: adrian.hunter@intel.com
TO: kan.liang@linux.intel.com
TO: james.clark@arm.com
TO: linux-perf-users@vger.kernel.org
TO: linux-kernel@vger.kernel.org
CC: yangjihong@bytedance.com

Hi Yang,

kernel test robot noticed the following build errors:

[auto build test ERROR on perf-tools-next/perf-tools-next]
[also build test ERROR on tip/perf/core perf-tools/perf-tools linus/master v6.9-rc2 next-20240402]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Yang-Jihong/perf-sched-timehist-Fix-g-call-graph-option-failure/20240401-143023
base:   https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git perf-tools-next
patch link:    https://lore.kernel.org/r/20240401062724.1006010-3-yangjihong%40bytedance.com
patch subject: [PATCH v2 2/2] perf evsel: Use evsel__name_is() helper
:::::: branch date: 32 hours ago
:::::: commit date: 32 hours ago
compiler: gcc-13 (Ubuntu 13.2.0-4ubuntu3) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240402/202404022240.nFyJIMwC-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/r/202404022240.nFyJIMwC-lkp@intel.com/

All errors (new ones prefixed by >>):

   Makefile.config:1095: No libcapstone found, disables disasm engine support for 'perf script', please install libcapstone-dev/capstone-devel
   Makefile.config:1160: libpfm4 not found, disables libpfm4 support. Please install libpfm4-dev
     PERF_VERSION = 6.8.g91c361385eb9
   builtin-trace.c: In function 'cmd_trace':
>> builtin-trace.c:4919:90: error: macro "assert" passed 2 arguments, but takes just 1
    4919 |         assert(evsel__name_is(trace.syscalls.events.bpf_output), "__augmented_syscalls__");
         |                                                                                          ^
   In file included from tools/include/linux/kernel.h:7,
                    from tools/include/linux/refcount.h:42,
                    from util/cgroup.h:6,
                    from builtin-trace.c:29:
   /usr/include/assert.h:50: note: macro "assert" defined here
      50 | # define assert(expr)           (__ASSERT_VOID_CAST (0))
         | 
>> builtin-trace.c:4919:9: error: 'assert' undeclared (first use in this function)
    4919 |         assert(evsel__name_is(trace.syscalls.events.bpf_output), "__augmented_syscalls__");
         |         ^~~~~~
   builtin-trace.c:915:1: note: 'assert' is defined in header '<assert.h>'; did you forget to '#include <assert.h>'?
     914 | #include "trace/beauty/waitid_options.c"
     +++ |+#include <assert.h>
     915 | 
   builtin-trace.c:4919:9: note: each undeclared identifier is reported only once for each function it appears in
    4919 |         assert(evsel__name_is(trace.syscalls.events.bpf_output), "__augmented_syscalls__");
         |         ^~~~~~
   make[6]: *** [tools/build/Makefile.build:105: tools/perf/builtin-trace.o] Error 1
   make[6]: *** Waiting for unfinished jobs....
   make[5]: *** [Makefile.perf:727: tools/perf/perf-in.o] Error 2
   make[5]: *** Waiting for unfinished jobs....
   make[4]: *** [Makefile.perf:264: sub-make] Error 2
   make[3]: *** [Makefile:70: all] Error 2

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki