* Re: [PATCH 1/1] perf evsel: Introduce evsel__name_is() method to check if the evsel name is equal to a given string
[not found] <ZEGLM8VehJbS0gP2@kernel.org>
@ 2023-04-20 19:10 ` Arnaldo Carvalho de Melo
2023-04-20 19:28 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-04-20 19:10 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Linux Kernel Mailing List, Adrian Hunter, Ian Rogers, Jiri Olsa,
Liang, Kan, Namhyung Kim
Em Thu, Apr 20, 2023 at 03:57:55PM -0300, Arnaldo Carvalho de Melo escreveu:
> This makes the logic a bit clear by avoiding the !strcmp() pattern and
> also a way to intercept the pointer if we need to do extra validation on
> it or to do lazy setting of evsel->name via evsel__name(evsel).
+ this, looking if there are others...
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index d3cbee7460fcc48e..e95a545f2c600810 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -2169,10 +2169,8 @@ static void setup_system_wide(int forks)
struct evsel *counter;
evlist__for_each_entry(evsel_list, counter) {
- if (!counter->core.requires_cpu &&
- strcmp(counter->name, "duration_time")) {
+ if (!counter->core.requires_cpu && evsel__name_is(counter, "duration_time"))
return;
- }
}
if (evsel_list->core.nr_entries)
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] perf evsel: Introduce evsel__name_is() method to check if the evsel name is equal to a given string
2023-04-20 19:10 ` [PATCH 1/1] perf evsel: Introduce evsel__name_is() method to check if the evsel name is equal to a given string Arnaldo Carvalho de Melo
@ 2023-04-20 19:28 ` Arnaldo Carvalho de Melo
2023-04-20 21:16 ` Liang, Kan
0 siblings, 1 reply; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-04-20 19:28 UTC (permalink / raw)
To: Linux Kernel Mailing List
Cc: Adrian Hunter, Ian Rogers, Jiri Olsa, Liang, Kan, Namhyung Kim
Em Thu, Apr 20, 2023 at 04:10:30PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Apr 20, 2023 at 03:57:55PM -0300, Arnaldo Carvalho de Melo escreveu:
> > This makes the logic a bit clear by avoiding the !strcmp() pattern and
> > also a way to intercept the pointer if we need to do extra validation on
> > it or to do lazy setting of evsel->name via evsel__name(evsel).
>
> + this, looking if there are others...
Somehow the first message didn't go thru, so below is the combined
patch, this is an effort to avoid accessing evsel->name directly as the
preferred way to get an evsel name is evsel__name(), so looking for
direct access and providing accessors that avoid that.
- Arnaldo
From e60455d6a4e35ba0c376966443294586a1adc3ec Mon Sep 17 00:00:00 2001
From: Arnaldo Carvalho de Melo <acme@redhat.com>
Date: Thu, 20 Apr 2023 15:54:11 -0300
Subject: [PATCH 1/1] perf evsel: Introduce evsel__name_is() method to check if
the evsel name is equal to a given string
This makes the logic a bit clear by avoiding the !strcmp() pattern and
also a way to intercept the pointer if we need to do extra validation on
it or to do lazy setting of evsel->name via evsel__name(evsel).
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Ian Rogers <irogers@google.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: "Liang, Kan" <kan.liang@linux.intel.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Link: https://lore.kernel.org/lkml/ZEGLM8VehJbS0gP2@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/arch/arm64/util/kvm-stat.c | 4 ++--
tools/perf/arch/powerpc/util/kvm-stat.c | 4 ++--
tools/perf/arch/x86/util/kvm-stat.c | 8 ++++----
tools/perf/builtin-kvm.c | 6 +++---
tools/perf/builtin-stat.c | 2 +-
tools/perf/tests/expand-cgroup.c | 2 +-
tools/perf/tests/parse-events.c | 12 ++++++------
tools/perf/tests/parse-metric.c | 2 +-
tools/perf/tests/pmu-events.c | 2 +-
tools/perf/util/evsel.c | 7 ++++++-
tools/perf/util/evsel.h | 1 +
11 files changed, 28 insertions(+), 22 deletions(-)
diff --git a/tools/perf/arch/arm64/util/kvm-stat.c b/tools/perf/arch/arm64/util/kvm-stat.c
index 72ca9bb45804d109..6611aa21cba957d9 100644
--- a/tools/perf/arch/arm64/util/kvm-stat.c
+++ b/tools/perf/arch/arm64/util/kvm-stat.c
@@ -44,14 +44,14 @@ static bool event_begin(struct evsel *evsel,
struct perf_sample *sample __maybe_unused,
struct event_key *key __maybe_unused)
{
- return !strcmp(evsel->name, kvm_entry_trace);
+ return evsel__name_is(evsel, kvm_entry_trace);
}
static bool event_end(struct evsel *evsel,
struct perf_sample *sample,
struct event_key *key)
{
- if (!strcmp(evsel->name, kvm_exit_trace)) {
+ if (evsel__name_is(evsel, kvm_exit_trace)) {
event_get_key(evsel, sample, key);
return true;
}
diff --git a/tools/perf/arch/powerpc/util/kvm-stat.c b/tools/perf/arch/powerpc/util/kvm-stat.c
index d04a08c9fd19c58c..ea1220d66b6758b2 100644
--- a/tools/perf/arch/powerpc/util/kvm-stat.c
+++ b/tools/perf/arch/powerpc/util/kvm-stat.c
@@ -60,13 +60,13 @@ static bool hcall_event_end(struct evsel *evsel,
struct perf_sample *sample __maybe_unused,
struct event_key *key __maybe_unused)
{
- return (!strcmp(evsel->name, kvm_events_tp[3]));
+ return (evsel__name_is(evsel, kvm_events_tp[3]));
}
static bool hcall_event_begin(struct evsel *evsel,
struct perf_sample *sample, struct event_key *key)
{
- if (!strcmp(evsel->name, kvm_events_tp[2])) {
+ if (evsel__name_is(evsel, kvm_events_tp[2])) {
hcall_event_get_key(evsel, sample, key);
return true;
}
diff --git a/tools/perf/arch/x86/util/kvm-stat.c b/tools/perf/arch/x86/util/kvm-stat.c
index ef513def03bac71c..424716518b755915 100644
--- a/tools/perf/arch/x86/util/kvm-stat.c
+++ b/tools/perf/arch/x86/util/kvm-stat.c
@@ -46,7 +46,7 @@ static bool mmio_event_begin(struct evsel *evsel,
return true;
/* MMIO write begin event in kernel. */
- if (!strcmp(evsel->name, "kvm:kvm_mmio") &&
+ if (evsel__name_is(evsel, "kvm:kvm_mmio") &&
evsel__intval(evsel, sample, "type") == KVM_TRACE_MMIO_WRITE) {
mmio_event_get_key(evsel, sample, key);
return true;
@@ -63,7 +63,7 @@ static bool mmio_event_end(struct evsel *evsel, struct perf_sample *sample,
return true;
/* MMIO read end event in kernel.*/
- if (!strcmp(evsel->name, "kvm:kvm_mmio") &&
+ if (evsel__name_is(evsel, "kvm:kvm_mmio") &&
evsel__intval(evsel, sample, "type") == KVM_TRACE_MMIO_READ) {
mmio_event_get_key(evsel, sample, key);
return true;
@@ -101,7 +101,7 @@ static bool ioport_event_begin(struct evsel *evsel,
struct perf_sample *sample,
struct event_key *key)
{
- if (!strcmp(evsel->name, "kvm:kvm_pio")) {
+ if (evsel__name_is(evsel, "kvm:kvm_pio")) {
ioport_event_get_key(evsel, sample, key);
return true;
}
@@ -145,7 +145,7 @@ static bool msr_event_begin(struct evsel *evsel,
struct perf_sample *sample,
struct event_key *key)
{
- if (!strcmp(evsel->name, "kvm:kvm_msr")) {
+ if (evsel__name_is(evsel, "kvm:kvm_msr")) {
msr_event_get_key(evsel, sample, key);
return true;
}
diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index 747d19336340f28f..71165036e4cac89b 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -625,7 +625,7 @@ void exit_event_get_key(struct evsel *evsel,
bool kvm_exit_event(struct evsel *evsel)
{
- return !strcmp(evsel->name, kvm_exit_trace);
+ return evsel__name_is(evsel, kvm_exit_trace);
}
bool exit_event_begin(struct evsel *evsel,
@@ -641,7 +641,7 @@ bool exit_event_begin(struct evsel *evsel,
bool kvm_entry_event(struct evsel *evsel)
{
- return !strcmp(evsel->name, kvm_entry_trace);
+ return evsel__name_is(evsel, kvm_entry_trace);
}
bool exit_event_end(struct evsel *evsel,
@@ -878,7 +878,7 @@ static bool is_child_event(struct perf_kvm_stat *kvm,
return false;
for (; child_ops->name; child_ops++) {
- if (!strcmp(evsel->name, child_ops->name)) {
+ if (evsel__name_is(evsel, child_ops->name)) {
child_ops->get_key(evsel, sample, key);
return true;
}
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index d3cbee7460fcc48e..efda63f6bf329b51 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -2170,7 +2170,7 @@ static void setup_system_wide(int forks)
evlist__for_each_entry(evsel_list, counter) {
if (!counter->core.requires_cpu &&
- strcmp(counter->name, "duration_time")) {
+ !evsel__name_is(counter, "duration_time")) {
return;
}
}
diff --git a/tools/perf/tests/expand-cgroup.c b/tools/perf/tests/expand-cgroup.c
index ec340880a848d907..9c1a1f18db750607 100644
--- a/tools/perf/tests/expand-cgroup.c
+++ b/tools/perf/tests/expand-cgroup.c
@@ -61,7 +61,7 @@ static int test_expand_events(struct evlist *evlist,
i = 0;
evlist__for_each_entry(evlist, evsel) {
- if (strcmp(evsel->name, ev_name[i % nr_events])) {
+ if (!evsel__name_is(evsel, ev_name[i % nr_events])) {
pr_debug("event name doesn't match:\n");
pr_debug(" evsel[%d]: %s\n expected: %s\n",
i, evsel->name, ev_name[i % nr_events]);
diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
index 6eb1400443adddee..8068cfd89b84f723 100644
--- a/tools/perf/tests/parse-events.c
+++ b/tools/perf/tests/parse-events.c
@@ -1401,7 +1401,7 @@ static int test__checkevent_config_symbol(struct evlist *evlist)
{
struct evsel *evsel = evlist__first(evlist);
- TEST_ASSERT_VAL("wrong name setting", strcmp(evsel->name, "insn") == 0);
+ TEST_ASSERT_VAL("wrong name setting", evsel__name_is(evsel, "insn"));
return TEST_OK;
}
@@ -1409,7 +1409,7 @@ static int test__checkevent_config_raw(struct evlist *evlist)
{
struct evsel *evsel = evlist__first(evlist);
- TEST_ASSERT_VAL("wrong name setting", strcmp(evsel->name, "rawpmu") == 0);
+ TEST_ASSERT_VAL("wrong name setting", evsel__name_is(evsel, "rawpmu"));
return TEST_OK;
}
@@ -1417,7 +1417,7 @@ static int test__checkevent_config_num(struct evlist *evlist)
{
struct evsel *evsel = evlist__first(evlist);
- TEST_ASSERT_VAL("wrong name setting", strcmp(evsel->name, "numpmu") == 0);
+ TEST_ASSERT_VAL("wrong name setting", evsel__name_is(evsel, "numpmu"));
return TEST_OK;
}
@@ -1425,7 +1425,7 @@ static int test__checkevent_config_cache(struct evlist *evlist)
{
struct evsel *evsel = evlist__first(evlist);
- TEST_ASSERT_VAL("wrong name setting", strcmp(evsel->name, "cachepmu") == 0);
+ TEST_ASSERT_VAL("wrong name setting", evsel__name_is(evsel, "cachepmu"));
return TEST_OK;
}
@@ -1438,7 +1438,7 @@ static int test__intel_pt(struct evlist *evlist)
{
struct evsel *evsel = evlist__first(evlist);
- TEST_ASSERT_VAL("wrong name setting", strcmp(evsel->name, "intel_pt//u") == 0);
+ TEST_ASSERT_VAL("wrong name setting", evsel__name_is(evsel, "intel_pt//u"));
return TEST_OK;
}
@@ -1446,7 +1446,7 @@ static int test__checkevent_complex_name(struct evlist *evlist)
{
struct evsel *evsel = evlist__first(evlist);
- TEST_ASSERT_VAL("wrong complex name parsing", strcmp(evsel->name, "COMPLEX_CYCLES_NAME:orig=cycles,desc=chip-clock-ticks") == 0);
+ TEST_ASSERT_VAL("wrong complex name parsing", evsel__name_is(evsel, "COMPLEX_CYCLES_NAME:orig=cycles,desc=chip-clock-ticks"));
return TEST_OK;
}
diff --git a/tools/perf/tests/parse-metric.c b/tools/perf/tests/parse-metric.c
index c43b056f9fa395f4..1185b79e6274886e 100644
--- a/tools/perf/tests/parse-metric.c
+++ b/tools/perf/tests/parse-metric.c
@@ -39,7 +39,7 @@ static void load_runtime_stat(struct evlist *evlist, struct value *vals)
evlist__for_each_entry(evlist, evsel) {
count = find_value(evsel->name, vals);
evsel->stats->aggr->counts.val = count;
- if (!strcmp(evsel->name, "duration_time"))
+ if (evsel__name_is(evsel, "duration_time"))
update_stats(&walltime_nsecs_stats, count);
}
}
diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
index 7f8e864525271483..1dff863b9711cf6d 100644
--- a/tools/perf/tests/pmu-events.c
+++ b/tools/perf/tests/pmu-events.c
@@ -866,7 +866,7 @@ static int test__parsing_callback(const struct pmu_metric *pm,
evlist__alloc_aggr_stats(evlist, 1);
evlist__for_each_entry(evlist, evsel) {
evsel->stats->aggr->counts.val = k;
- if (!strcmp(evsel->name, "duration_time"))
+ if (evsel__name_is(evsel, "duration_time"))
update_stats(&walltime_nsecs_stats, k);
k++;
}
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index a85a987128aad281..81b854650160c2b0 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -821,6 +821,11 @@ const char *evsel__name(struct evsel *evsel)
return "unknown";
}
+bool evsel__name_is(struct evsel *evsel, const char *name)
+{
+ return !strcmp(evsel->name, name);
+}
+
const char *evsel__group_pmu_name(const struct evsel *evsel)
{
const struct evsel *leader;
@@ -1146,7 +1151,7 @@ static void evsel__set_default_freq_period(struct record_opts *opts,
static bool evsel__is_offcpu_event(struct evsel *evsel)
{
- return evsel__is_bpf_output(evsel) && !strcmp(evsel->name, OFFCPU_EVENT);
+ return evsel__is_bpf_output(evsel) && evsel__name_is(evsel, OFFCPU_EVENT);
}
/*
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 68072ec655ce9fff..1e5d640e4a9bd0f1 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -282,6 +282,7 @@ int arch_evsel__hw_name(struct evsel *evsel, char *bf, size_t size);
int __evsel__hw_cache_type_op_res_name(u8 type, u8 op, u8 result, char *bf, size_t size);
const char *evsel__name(struct evsel *evsel);
+bool evsel__name_is(struct evsel *evsel, const char *name);
const char *evsel__group_pmu_name(const struct evsel *evsel);
const char *evsel__metric_id(const struct evsel *evsel);
--
2.39.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] perf evsel: Introduce evsel__name_is() method to check if the evsel name is equal to a given string
2023-04-20 19:28 ` Arnaldo Carvalho de Melo
@ 2023-04-20 21:16 ` Liang, Kan
2023-04-24 17:25 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 6+ messages in thread
From: Liang, Kan @ 2023-04-20 21:16 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Linux Kernel Mailing List
Cc: Adrian Hunter, Ian Rogers, Jiri Olsa, Namhyung Kim
On 2023-04-20 3:28 p.m., Arnaldo Carvalho de Melo wrote:
> Em Thu, Apr 20, 2023 at 04:10:30PM -0300, Arnaldo Carvalho de Melo escreveu:
>> Em Thu, Apr 20, 2023 at 03:57:55PM -0300, Arnaldo Carvalho de Melo escreveu:
>>> This makes the logic a bit clear by avoiding the !strcmp() pattern and
>>> also a way to intercept the pointer if we need to do extra validation on
>>> it or to do lazy setting of evsel->name via evsel__name(evsel).
>>
>> + this, looking if there are others...
>
> Somehow the first message didn't go thru, so below is the combined
> patch, this is an effort to avoid accessing evsel->name directly as the
> preferred way to get an evsel name is evsel__name(), so looking for
> direct access and providing accessors that avoid that.
One more
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 2260e27adf44..3a960a3f6962 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -467,7 +467,7 @@ static int evsel__strcmp(struct evsel *pos, char
*evsel_name)
return 0;
if (evsel__is_dummy_event(pos))
return 1;
- return strcmp(pos->name, evsel_name);
+ return !evsel__name_is(pos, evsel_name);
}
static int evlist__is_enabled(struct evlist *evlist)
>
> - Arnaldo
>
> From e60455d6a4e35ba0c376966443294586a1adc3ec Mon Sep 17 00:00:00 2001
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> Date: Thu, 20 Apr 2023 15:54:11 -0300
> Subject: [PATCH 1/1] perf evsel: Introduce evsel__name_is() method to check if
> the evsel name is equal to a given string
>
> This makes the logic a bit clear by avoiding the !strcmp() pattern and
> also a way to intercept the pointer if we need to do extra validation on
> it or to do lazy setting of evsel->name via evsel__name(evsel).
>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: Ian Rogers <irogers@google.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: "Liang, Kan" <kan.liang@linux.intel.com>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Link: https://lore.kernel.org/lkml/ZEGLM8VehJbS0gP2@kernel.org
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
With the above one,
Reviewed-by: Kan Liang <kan.liang@linux.intel.com>
Thanks,
Kan
> ---
> tools/perf/arch/arm64/util/kvm-stat.c | 4 ++--
> tools/perf/arch/powerpc/util/kvm-stat.c | 4 ++--
> tools/perf/arch/x86/util/kvm-stat.c | 8 ++++----
> tools/perf/builtin-kvm.c | 6 +++---
> tools/perf/builtin-stat.c | 2 +-
> tools/perf/tests/expand-cgroup.c | 2 +-
> tools/perf/tests/parse-events.c | 12 ++++++------
> tools/perf/tests/parse-metric.c | 2 +-
> tools/perf/tests/pmu-events.c | 2 +-
> tools/perf/util/evsel.c | 7 ++++++-
> tools/perf/util/evsel.h | 1 +
> 11 files changed, 28 insertions(+), 22 deletions(-)
>
> diff --git a/tools/perf/arch/arm64/util/kvm-stat.c b/tools/perf/arch/arm64/util/kvm-stat.c
> index 72ca9bb45804d109..6611aa21cba957d9 100644
> --- a/tools/perf/arch/arm64/util/kvm-stat.c
> +++ b/tools/perf/arch/arm64/util/kvm-stat.c
> @@ -44,14 +44,14 @@ static bool event_begin(struct evsel *evsel,
> struct perf_sample *sample __maybe_unused,
> struct event_key *key __maybe_unused)
> {
> - return !strcmp(evsel->name, kvm_entry_trace);
> + return evsel__name_is(evsel, kvm_entry_trace);
> }
>
> static bool event_end(struct evsel *evsel,
> struct perf_sample *sample,
> struct event_key *key)
> {
> - if (!strcmp(evsel->name, kvm_exit_trace)) {
> + if (evsel__name_is(evsel, kvm_exit_trace)) {
> event_get_key(evsel, sample, key);
> return true;
> }
> diff --git a/tools/perf/arch/powerpc/util/kvm-stat.c b/tools/perf/arch/powerpc/util/kvm-stat.c
> index d04a08c9fd19c58c..ea1220d66b6758b2 100644
> --- a/tools/perf/arch/powerpc/util/kvm-stat.c
> +++ b/tools/perf/arch/powerpc/util/kvm-stat.c
> @@ -60,13 +60,13 @@ static bool hcall_event_end(struct evsel *evsel,
> struct perf_sample *sample __maybe_unused,
> struct event_key *key __maybe_unused)
> {
> - return (!strcmp(evsel->name, kvm_events_tp[3]));
> + return (evsel__name_is(evsel, kvm_events_tp[3]));
> }
>
> static bool hcall_event_begin(struct evsel *evsel,
> struct perf_sample *sample, struct event_key *key)
> {
> - if (!strcmp(evsel->name, kvm_events_tp[2])) {
> + if (evsel__name_is(evsel, kvm_events_tp[2])) {
> hcall_event_get_key(evsel, sample, key);
> return true;
> }
> diff --git a/tools/perf/arch/x86/util/kvm-stat.c b/tools/perf/arch/x86/util/kvm-stat.c
> index ef513def03bac71c..424716518b755915 100644
> --- a/tools/perf/arch/x86/util/kvm-stat.c
> +++ b/tools/perf/arch/x86/util/kvm-stat.c
> @@ -46,7 +46,7 @@ static bool mmio_event_begin(struct evsel *evsel,
> return true;
>
> /* MMIO write begin event in kernel. */
> - if (!strcmp(evsel->name, "kvm:kvm_mmio") &&
> + if (evsel__name_is(evsel, "kvm:kvm_mmio") &&
> evsel__intval(evsel, sample, "type") == KVM_TRACE_MMIO_WRITE) {
> mmio_event_get_key(evsel, sample, key);
> return true;
> @@ -63,7 +63,7 @@ static bool mmio_event_end(struct evsel *evsel, struct perf_sample *sample,
> return true;
>
> /* MMIO read end event in kernel.*/
> - if (!strcmp(evsel->name, "kvm:kvm_mmio") &&
> + if (evsel__name_is(evsel, "kvm:kvm_mmio") &&
> evsel__intval(evsel, sample, "type") == KVM_TRACE_MMIO_READ) {
> mmio_event_get_key(evsel, sample, key);
> return true;
> @@ -101,7 +101,7 @@ static bool ioport_event_begin(struct evsel *evsel,
> struct perf_sample *sample,
> struct event_key *key)
> {
> - if (!strcmp(evsel->name, "kvm:kvm_pio")) {
> + if (evsel__name_is(evsel, "kvm:kvm_pio")) {
> ioport_event_get_key(evsel, sample, key);
> return true;
> }
> @@ -145,7 +145,7 @@ static bool msr_event_begin(struct evsel *evsel,
> struct perf_sample *sample,
> struct event_key *key)
> {
> - if (!strcmp(evsel->name, "kvm:kvm_msr")) {
> + if (evsel__name_is(evsel, "kvm:kvm_msr")) {
> msr_event_get_key(evsel, sample, key);
> return true;
> }
> diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
> index 747d19336340f28f..71165036e4cac89b 100644
> --- a/tools/perf/builtin-kvm.c
> +++ b/tools/perf/builtin-kvm.c
> @@ -625,7 +625,7 @@ void exit_event_get_key(struct evsel *evsel,
>
> bool kvm_exit_event(struct evsel *evsel)
> {
> - return !strcmp(evsel->name, kvm_exit_trace);
> + return evsel__name_is(evsel, kvm_exit_trace);
> }
>
> bool exit_event_begin(struct evsel *evsel,
> @@ -641,7 +641,7 @@ bool exit_event_begin(struct evsel *evsel,
>
> bool kvm_entry_event(struct evsel *evsel)
> {
> - return !strcmp(evsel->name, kvm_entry_trace);
> + return evsel__name_is(evsel, kvm_entry_trace);
> }
>
> bool exit_event_end(struct evsel *evsel,
> @@ -878,7 +878,7 @@ static bool is_child_event(struct perf_kvm_stat *kvm,
> return false;
>
> for (; child_ops->name; child_ops++) {
> - if (!strcmp(evsel->name, child_ops->name)) {
> + if (evsel__name_is(evsel, child_ops->name)) {
> child_ops->get_key(evsel, sample, key);
> return true;
> }
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index d3cbee7460fcc48e..efda63f6bf329b51 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -2170,7 +2170,7 @@ static void setup_system_wide(int forks)
>
> evlist__for_each_entry(evsel_list, counter) {
> if (!counter->core.requires_cpu &&
> - strcmp(counter->name, "duration_time")) {
> + !evsel__name_is(counter, "duration_time")) {
> return;
> }
> }
> diff --git a/tools/perf/tests/expand-cgroup.c b/tools/perf/tests/expand-cgroup.c
> index ec340880a848d907..9c1a1f18db750607 100644
> --- a/tools/perf/tests/expand-cgroup.c
> +++ b/tools/perf/tests/expand-cgroup.c
> @@ -61,7 +61,7 @@ static int test_expand_events(struct evlist *evlist,
>
> i = 0;
> evlist__for_each_entry(evlist, evsel) {
> - if (strcmp(evsel->name, ev_name[i % nr_events])) {
> + if (!evsel__name_is(evsel, ev_name[i % nr_events])) {
> pr_debug("event name doesn't match:\n");
> pr_debug(" evsel[%d]: %s\n expected: %s\n",
> i, evsel->name, ev_name[i % nr_events]);
> diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-events.c
> index 6eb1400443adddee..8068cfd89b84f723 100644
> --- a/tools/perf/tests/parse-events.c
> +++ b/tools/perf/tests/parse-events.c
> @@ -1401,7 +1401,7 @@ static int test__checkevent_config_symbol(struct evlist *evlist)
> {
> struct evsel *evsel = evlist__first(evlist);
>
> - TEST_ASSERT_VAL("wrong name setting", strcmp(evsel->name, "insn") == 0);
> + TEST_ASSERT_VAL("wrong name setting", evsel__name_is(evsel, "insn"));
> return TEST_OK;
> }
>
> @@ -1409,7 +1409,7 @@ static int test__checkevent_config_raw(struct evlist *evlist)
> {
> struct evsel *evsel = evlist__first(evlist);
>
> - TEST_ASSERT_VAL("wrong name setting", strcmp(evsel->name, "rawpmu") == 0);
> + TEST_ASSERT_VAL("wrong name setting", evsel__name_is(evsel, "rawpmu"));
> return TEST_OK;
> }
>
> @@ -1417,7 +1417,7 @@ static int test__checkevent_config_num(struct evlist *evlist)
> {
> struct evsel *evsel = evlist__first(evlist);
>
> - TEST_ASSERT_VAL("wrong name setting", strcmp(evsel->name, "numpmu") == 0);
> + TEST_ASSERT_VAL("wrong name setting", evsel__name_is(evsel, "numpmu"));
> return TEST_OK;
> }
>
> @@ -1425,7 +1425,7 @@ static int test__checkevent_config_cache(struct evlist *evlist)
> {
> struct evsel *evsel = evlist__first(evlist);
>
> - TEST_ASSERT_VAL("wrong name setting", strcmp(evsel->name, "cachepmu") == 0);
> + TEST_ASSERT_VAL("wrong name setting", evsel__name_is(evsel, "cachepmu"));
> return TEST_OK;
> }
>
> @@ -1438,7 +1438,7 @@ static int test__intel_pt(struct evlist *evlist)
> {
> struct evsel *evsel = evlist__first(evlist);
>
> - TEST_ASSERT_VAL("wrong name setting", strcmp(evsel->name, "intel_pt//u") == 0);
> + TEST_ASSERT_VAL("wrong name setting", evsel__name_is(evsel, "intel_pt//u"));
> return TEST_OK;
> }
>
> @@ -1446,7 +1446,7 @@ static int test__checkevent_complex_name(struct evlist *evlist)
> {
> struct evsel *evsel = evlist__first(evlist);
>
> - TEST_ASSERT_VAL("wrong complex name parsing", strcmp(evsel->name, "COMPLEX_CYCLES_NAME:orig=cycles,desc=chip-clock-ticks") == 0);
> + TEST_ASSERT_VAL("wrong complex name parsing", evsel__name_is(evsel, "COMPLEX_CYCLES_NAME:orig=cycles,desc=chip-clock-ticks"));
> return TEST_OK;
> }
>
> diff --git a/tools/perf/tests/parse-metric.c b/tools/perf/tests/parse-metric.c
> index c43b056f9fa395f4..1185b79e6274886e 100644
> --- a/tools/perf/tests/parse-metric.c
> +++ b/tools/perf/tests/parse-metric.c
> @@ -39,7 +39,7 @@ static void load_runtime_stat(struct evlist *evlist, struct value *vals)
> evlist__for_each_entry(evlist, evsel) {
> count = find_value(evsel->name, vals);
> evsel->stats->aggr->counts.val = count;
> - if (!strcmp(evsel->name, "duration_time"))
> + if (evsel__name_is(evsel, "duration_time"))
> update_stats(&walltime_nsecs_stats, count);
> }
> }
> diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
> index 7f8e864525271483..1dff863b9711cf6d 100644
> --- a/tools/perf/tests/pmu-events.c
> +++ b/tools/perf/tests/pmu-events.c
> @@ -866,7 +866,7 @@ static int test__parsing_callback(const struct pmu_metric *pm,
> evlist__alloc_aggr_stats(evlist, 1);
> evlist__for_each_entry(evlist, evsel) {
> evsel->stats->aggr->counts.val = k;
> - if (!strcmp(evsel->name, "duration_time"))
> + if (evsel__name_is(evsel, "duration_time"))
> update_stats(&walltime_nsecs_stats, k);
> k++;
> }
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index a85a987128aad281..81b854650160c2b0 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -821,6 +821,11 @@ const char *evsel__name(struct evsel *evsel)
> return "unknown";
> }
>
> +bool evsel__name_is(struct evsel *evsel, const char *name)
> +{
> + return !strcmp(evsel->name, name);
> +}
> +
> const char *evsel__group_pmu_name(const struct evsel *evsel)
> {
> const struct evsel *leader;
> @@ -1146,7 +1151,7 @@ static void evsel__set_default_freq_period(struct record_opts *opts,
>
> static bool evsel__is_offcpu_event(struct evsel *evsel)
> {
> - return evsel__is_bpf_output(evsel) && !strcmp(evsel->name, OFFCPU_EVENT);
> + return evsel__is_bpf_output(evsel) && evsel__name_is(evsel, OFFCPU_EVENT);
> }
>
> /*
> diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
> index 68072ec655ce9fff..1e5d640e4a9bd0f1 100644
> --- a/tools/perf/util/evsel.h
> +++ b/tools/perf/util/evsel.h
> @@ -282,6 +282,7 @@ int arch_evsel__hw_name(struct evsel *evsel, char *bf, size_t size);
>
> int __evsel__hw_cache_type_op_res_name(u8 type, u8 op, u8 result, char *bf, size_t size);
> const char *evsel__name(struct evsel *evsel);
> +bool evsel__name_is(struct evsel *evsel, const char *name);
> const char *evsel__group_pmu_name(const struct evsel *evsel);
> const char *evsel__metric_id(const struct evsel *evsel);
>
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] perf evsel: Introduce evsel__name_is() method to check if the evsel name is equal to a given string
2023-04-20 21:16 ` Liang, Kan
@ 2023-04-24 17:25 ` Arnaldo Carvalho de Melo
2023-04-24 17:27 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-04-24 17:25 UTC (permalink / raw)
To: Liang, Kan
Cc: Linux Kernel Mailing List, Adrian Hunter, Ian Rogers, Jiri Olsa,
Namhyung Kim
Em Thu, Apr 20, 2023 at 05:16:18PM -0400, Liang, Kan escreveu:
>
>
> On 2023-04-20 3:28 p.m., Arnaldo Carvalho de Melo wrote:
> > Em Thu, Apr 20, 2023 at 04:10:30PM -0300, Arnaldo Carvalho de Melo escreveu:
> >> Em Thu, Apr 20, 2023 at 03:57:55PM -0300, Arnaldo Carvalho de Melo escreveu:
> >>> This makes the logic a bit clear by avoiding the !strcmp() pattern and
> >>> also a way to intercept the pointer if we need to do extra validation on
> >>> it or to do lazy setting of evsel->name via evsel__name(evsel).
> >>
> >> + this, looking if there are others...
> >
> > Somehow the first message didn't go thru, so below is the combined
> > patch, this is an effort to avoid accessing evsel->name directly as the
> > preferred way to get an evsel name is evsel__name(), so looking for
> > direct access and providing accessors that avoid that.
>
> One more
>
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 2260e27adf44..3a960a3f6962 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -467,7 +467,7 @@ static int evsel__strcmp(struct evsel *pos, char
> *evsel_name)
> return 0;
> if (evsel__is_dummy_event(pos))
> return 1;
> - return strcmp(pos->name, evsel_name);
> + return !evsel__name_is(pos, evsel_name);
> }
>
> static int evlist__is_enabled(struct evlist *evlist)
Added
> >
> > From e60455d6a4e35ba0c376966443294586a1adc3ec Mon Sep 17 00:00:00 2001
> > From: Arnaldo Carvalho de Melo <acme@redhat.com>
> > Date: Thu, 20 Apr 2023 15:54:11 -0300
> > Subject: [PATCH 1/1] perf evsel: Introduce evsel__name_is() method to check if
> > the evsel name is equal to a given string
> >
> > This makes the logic a bit clear by avoiding the !strcmp() pattern and
> > also a way to intercept the pointer if we need to do extra validation on
> > it or to do lazy setting of evsel->name via evsel__name(evsel).
> >
> > Cc: Adrian Hunter <adrian.hunter@intel.com>
> > Cc: Ian Rogers <irogers@google.com>
> > Cc: Jiri Olsa <jolsa@kernel.org>
> > Cc: "Liang, Kan" <kan.liang@linux.intel.com>
> > Cc: Namhyung Kim <namhyung@kernel.org>
> > Link: https://lore.kernel.org/lkml/ZEGLM8VehJbS0gP2@kernel.org
> > Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> With the above one,
>
> Reviewed-by: Kan Liang <kan.liang@linux.intel.com>
Added these extra ones and actually made evsel__name_is() use
evsel__name().
Does your reviewed-by stands after these extra changes?
Thanks,
- Arnaldo
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 2260e27adf44c579..a0504316b06fbcba 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -467,7 +467,7 @@ static int evsel__strcmp(struct evsel *pos, char *evsel_name)
return 0;
if (evsel__is_dummy_event(pos))
return 1;
- return strcmp(pos->name, evsel_name);
+ return !evsel__name_is(pos, evsel_name);
}
static int evlist__is_enabled(struct evlist *evlist)
@@ -1706,7 +1706,7 @@ struct evsel *evlist__find_evsel_by_str(struct evlist *evlist, const char *str)
evlist__for_each_entry(evlist, evsel) {
if (!evsel->name)
continue;
- if (strcmp(str, evsel->name) == 0)
+ if (evsel__name_is(evsel, str))
return evsel;
}
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 81b854650160c2b0..356c07f03be6bfce 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -823,7 +823,7 @@ const char *evsel__name(struct evsel *evsel)
bool evsel__name_is(struct evsel *evsel, const char *name)
{
- return !strcmp(evsel->name, name);
+ return !strcmp(evsel__name(evsel), name);
}
const char *evsel__group_pmu_name(const struct evsel *evsel)
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 31b1cd0935e277ba..dae81d8e1769c763 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -2893,7 +2893,7 @@ static struct evsel *find_evsel(struct evlist *evlist, char *event_name)
full_name = !!strchr(event_name, ':');
evlist__for_each_entry(evlist, pos) {
/* case 2 */
- if (full_name && !strcmp(pos->name, event_name))
+ if (full_name && evsel__name_is(pos->name, event_name))
return pos;
/* case 3 */
if (!full_name && strstr(pos->name, event_name)) {
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] perf evsel: Introduce evsel__name_is() method to check if the evsel name is equal to a given string
2023-04-24 17:25 ` Arnaldo Carvalho de Melo
@ 2023-04-24 17:27 ` Arnaldo Carvalho de Melo
2023-04-24 17:50 ` Liang, Kan
0 siblings, 1 reply; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2023-04-24 17:27 UTC (permalink / raw)
To: Liang, Kan
Cc: Linux Kernel Mailing List, Adrian Hunter, Ian Rogers, Jiri Olsa,
Namhyung Kim
Em Mon, Apr 24, 2023 at 02:25:27PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Apr 20, 2023 at 05:16:18PM -0400, Liang, Kan escreveu:
> >
> >
> > On 2023-04-20 3:28 p.m., Arnaldo Carvalho de Melo wrote:
> > > Em Thu, Apr 20, 2023 at 04:10:30PM -0300, Arnaldo Carvalho de Melo escreveu:
> > >> Em Thu, Apr 20, 2023 at 03:57:55PM -0300, Arnaldo Carvalho de Melo escreveu:
> > >>> This makes the logic a bit clear by avoiding the !strcmp() pattern and
> > >>> also a way to intercept the pointer if we need to do extra validation on
> > >>> it or to do lazy setting of evsel->name via evsel__name(evsel).
> > >>
> > >> + this, looking if there are others...
> > >
> > > Somehow the first message didn't go thru, so below is the combined
> > > patch, this is an effort to avoid accessing evsel->name directly as the
> > > preferred way to get an evsel name is evsel__name(), so looking for
> > > direct access and providing accessors that avoid that.
> >
> > One more
> >
> > diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> > index 2260e27adf44..3a960a3f6962 100644
> > --- a/tools/perf/util/evlist.c
> > +++ b/tools/perf/util/evlist.c
> > @@ -467,7 +467,7 @@ static int evsel__strcmp(struct evsel *pos, char
> > *evsel_name)
> > return 0;
> > if (evsel__is_dummy_event(pos))
> > return 1;
> > - return strcmp(pos->name, evsel_name);
> > + return !evsel__name_is(pos, evsel_name);
> > }
> >
> > static int evlist__is_enabled(struct evlist *evlist)
>
> Added
>
> > >
> > > From e60455d6a4e35ba0c376966443294586a1adc3ec Mon Sep 17 00:00:00 2001
> > > From: Arnaldo Carvalho de Melo <acme@redhat.com>
> > > Date: Thu, 20 Apr 2023 15:54:11 -0300
> > > Subject: [PATCH 1/1] perf evsel: Introduce evsel__name_is() method to check if
> > > the evsel name is equal to a given string
> > >
> > > This makes the logic a bit clear by avoiding the !strcmp() pattern and
> > > also a way to intercept the pointer if we need to do extra validation on
> > > it or to do lazy setting of evsel->name via evsel__name(evsel).
> > >
> > > Cc: Adrian Hunter <adrian.hunter@intel.com>
> > > Cc: Ian Rogers <irogers@google.com>
> > > Cc: Jiri Olsa <jolsa@kernel.org>
> > > Cc: "Liang, Kan" <kan.liang@linux.intel.com>
> > > Cc: Namhyung Kim <namhyung@kernel.org>
> > > Link: https://lore.kernel.org/lkml/ZEGLM8VehJbS0gP2@kernel.org
> > > Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> >
> > With the above one,
> >
> > Reviewed-by: Kan Liang <kan.liang@linux.intel.com>
>
> Added these extra ones and actually made evsel__name_is() use
> evsel__name().
>
> Does your reviewed-by stands after these extra changes?
oops, fix that pos->name leftover:
diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 2260e27adf44c579..a0504316b06fbcba 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -467,7 +467,7 @@ static int evsel__strcmp(struct evsel *pos, char *evsel_name)
return 0;
if (evsel__is_dummy_event(pos))
return 1;
- return strcmp(pos->name, evsel_name);
+ return !evsel__name_is(pos, evsel_name);
}
static int evlist__is_enabled(struct evlist *evlist)
@@ -1706,7 +1706,7 @@ struct evsel *evlist__find_evsel_by_str(struct evlist *evlist, const char *str)
evlist__for_each_entry(evlist, evsel) {
if (!evsel->name)
continue;
- if (strcmp(str, evsel->name) == 0)
+ if (evsel__name_is(evsel, str))
return evsel;
}
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 81b854650160c2b0..356c07f03be6bfce 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -823,7 +823,7 @@ const char *evsel__name(struct evsel *evsel)
bool evsel__name_is(struct evsel *evsel, const char *name)
{
- return !strcmp(evsel->name, name);
+ return !strcmp(evsel__name(evsel), name);
}
const char *evsel__group_pmu_name(const struct evsel *evsel)
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 31b1cd0935e277ba..650cd8df40412a38 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -2893,7 +2893,7 @@ static struct evsel *find_evsel(struct evlist *evlist, char *event_name)
full_name = !!strchr(event_name, ':');
evlist__for_each_entry(evlist, pos) {
/* case 2 */
- if (full_name && !strcmp(pos->name, event_name))
+ if (full_name && evsel__name_is(pos, event_name))
return pos;
/* case 3 */
if (!full_name && strstr(pos->name, event_name)) {
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/1] perf evsel: Introduce evsel__name_is() method to check if the evsel name is equal to a given string
2023-04-24 17:27 ` Arnaldo Carvalho de Melo
@ 2023-04-24 17:50 ` Liang, Kan
0 siblings, 0 replies; 6+ messages in thread
From: Liang, Kan @ 2023-04-24 17:50 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Linux Kernel Mailing List, Adrian Hunter, Ian Rogers, Jiri Olsa,
Namhyung Kim
On 2023-04-24 1:27 p.m., Arnaldo Carvalho de Melo wrote:
> Em Mon, Apr 24, 2023 at 02:25:27PM -0300, Arnaldo Carvalho de Melo escreveu:
>> Em Thu, Apr 20, 2023 at 05:16:18PM -0400, Liang, Kan escreveu:
>>>
>>>
>>> On 2023-04-20 3:28 p.m., Arnaldo Carvalho de Melo wrote:
>>>> Em Thu, Apr 20, 2023 at 04:10:30PM -0300, Arnaldo Carvalho de Melo escreveu:
>>>>> Em Thu, Apr 20, 2023 at 03:57:55PM -0300, Arnaldo Carvalho de Melo escreveu:
>>>>>> This makes the logic a bit clear by avoiding the !strcmp() pattern and
>>>>>> also a way to intercept the pointer if we need to do extra validation on
>>>>>> it or to do lazy setting of evsel->name via evsel__name(evsel).
>>>>>
>>>>> + this, looking if there are others...
>>>>
>>>> Somehow the first message didn't go thru, so below is the combined
>>>> patch, this is an effort to avoid accessing evsel->name directly as the
>>>> preferred way to get an evsel name is evsel__name(), so looking for
>>>> direct access and providing accessors that avoid that.
>>>
>>> One more
>>>
>>> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
>>> index 2260e27adf44..3a960a3f6962 100644
>>> --- a/tools/perf/util/evlist.c
>>> +++ b/tools/perf/util/evlist.c
>>> @@ -467,7 +467,7 @@ static int evsel__strcmp(struct evsel *pos, char
>>> *evsel_name)
>>> return 0;
>>> if (evsel__is_dummy_event(pos))
>>> return 1;
>>> - return strcmp(pos->name, evsel_name);
>>> + return !evsel__name_is(pos, evsel_name);
>>> }
>>>
>>> static int evlist__is_enabled(struct evlist *evlist)
>>
>> Added
>>
>>>>
>>>> From e60455d6a4e35ba0c376966443294586a1adc3ec Mon Sep 17 00:00:00 2001
>>>> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>>>> Date: Thu, 20 Apr 2023 15:54:11 -0300
>>>> Subject: [PATCH 1/1] perf evsel: Introduce evsel__name_is() method to check if
>>>> the evsel name is equal to a given string
>>>>
>>>> This makes the logic a bit clear by avoiding the !strcmp() pattern and
>>>> also a way to intercept the pointer if we need to do extra validation on
>>>> it or to do lazy setting of evsel->name via evsel__name(evsel).
>>>>
>>>> Cc: Adrian Hunter <adrian.hunter@intel.com>
>>>> Cc: Ian Rogers <irogers@google.com>
>>>> Cc: Jiri Olsa <jolsa@kernel.org>
>>>> Cc: "Liang, Kan" <kan.liang@linux.intel.com>
>>>> Cc: Namhyung Kim <namhyung@kernel.org>
>>>> Link: https://lore.kernel.org/lkml/ZEGLM8VehJbS0gP2@kernel.org
>>>> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>>>
>>> With the above one,
>>>
>>> Reviewed-by: Kan Liang <kan.liang@linux.intel.com>
>>
>> Added these extra ones and actually made evsel__name_is() use
>> evsel__name().
>>
>> Does your reviewed-by stands after these extra changes?
>
Yes, the below extra changes look good to me as well.
Thanks,
Kan
>
> oops, fix that pos->name leftover:
>
>
> diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
> index 2260e27adf44c579..a0504316b06fbcba 100644
> --- a/tools/perf/util/evlist.c
> +++ b/tools/perf/util/evlist.c
> @@ -467,7 +467,7 @@ static int evsel__strcmp(struct evsel *pos, char *evsel_name)
> return 0;
> if (evsel__is_dummy_event(pos))
> return 1;
> - return strcmp(pos->name, evsel_name);
> + return !evsel__name_is(pos, evsel_name);
> }
>
> static int evlist__is_enabled(struct evlist *evlist)
> @@ -1706,7 +1706,7 @@ struct evsel *evlist__find_evsel_by_str(struct evlist *evlist, const char *str)
> evlist__for_each_entry(evlist, evsel) {
> if (!evsel->name)
> continue;
> - if (strcmp(str, evsel->name) == 0)
> + if (evsel__name_is(evsel, str))
> return evsel;
> }
>
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 81b854650160c2b0..356c07f03be6bfce 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -823,7 +823,7 @@ const char *evsel__name(struct evsel *evsel)
>
> bool evsel__name_is(struct evsel *evsel, const char *name)
> {
> - return !strcmp(evsel->name, name);
> + return !strcmp(evsel__name(evsel), name);
> }
>
> const char *evsel__group_pmu_name(const struct evsel *evsel)
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index 31b1cd0935e277ba..650cd8df40412a38 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -2893,7 +2893,7 @@ static struct evsel *find_evsel(struct evlist *evlist, char *event_name)
> full_name = !!strchr(event_name, ':');
> evlist__for_each_entry(evlist, pos) {
> /* case 2 */
> - if (full_name && !strcmp(pos->name, event_name))
> + if (full_name && evsel__name_is(pos, event_name))
> return pos;
> /* case 3 */
> if (!full_name && strstr(pos->name, event_name)) {
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-04-24 17:50 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <ZEGLM8VehJbS0gP2@kernel.org>
2023-04-20 19:10 ` [PATCH 1/1] perf evsel: Introduce evsel__name_is() method to check if the evsel name is equal to a given string Arnaldo Carvalho de Melo
2023-04-20 19:28 ` Arnaldo Carvalho de Melo
2023-04-20 21:16 ` Liang, Kan
2023-04-24 17:25 ` Arnaldo Carvalho de Melo
2023-04-24 17:27 ` Arnaldo Carvalho de Melo
2023-04-24 17:50 ` Liang, Kan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox