Received-SPF: pass (google.com: domain of linux-kernel+bounces-123217-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240328055857.383180-1-yangjihong@bytedance.com> <20240328055857.383180-2-yangjihong@bytedance.com>
In-Reply-To: <20240328055857.383180-2-yangjihong@bytedance.com>
From: Ian Rogers <irogers@google.com>
Date: Thu, 28 Mar 2024 09:06:28 -0700
Message-ID: <CAP-5=fWA0EmxfxHOL2N2j=itUu_-wPxugB1Am6zi2pRAxBSFtg@mail.gmail.com>
Subject: Re: [PATCH 2/2] perf evsel: Use evsel__name_is() helper
To: Yang Jihong <yangjihong@bytedance.com>
Cc: peterz@infradead.org, mingo@redhat.com, acme@kernel.org, 
	namhyung@kernel.org, mark.rutland@arm.com, alexander.shishkin@linux.intel.com, 
	jolsa@kernel.org, adrian.hunter@intel.com, kan.liang@linux.intel.com, 
	james.clark@arm.com, linux-perf-users@vger.kernel.org, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Mar 27, 2024 at 10:59=E2=80=AFPM Yang Jihong <yangjihong@bytedance.=
com> wrote:
>
> 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>

Thanks,
Ian

> ---
>  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 =3D 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 *samp=
le,
>                            struct evsel *evsel)
>  {
>         /* pid 0 =3D=3D swapper =3D=3D idle task */
> -       if (strcmp(evsel__name(evsel), "sched:sched_switch") =3D=3D 0)
> +       if (evsel__name_is(evsel, "sched:sched_switch"))
>                 return evsel__intval(evsel, sample, "prev_pid") =3D=3D 0;
>
>         return sample->pid =3D=3D 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 =3D true;
>                 else if (evsel__intval(evsel, sample, "prev_pid") !=3D 0 =
&&
>                          evsel__intval(evsel, sample, "next_pid") !=3D 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 *scr=
iptname,
>
>                         match =3D 0;
>                         evlist__for_each_entry(session->evlist, pos) {
> -                               if (!strcmp(evsel__name(pos), evname)) {
> +                               if (evsel__name_is(pos, evname)) {
>                                         match =3D 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 =3D evlist__last(trace.evlist);
> -       assert(!strcmp(evsel__name(trace.syscalls.events.bpf_output), "__=
augmented_syscalls__"));
> +       assert(evsel__name_is(trace.syscalls.events.bpf_output), "__augme=
nted_syscalls__");
>  skip_augmentation:
>  #endif
>         err =3D -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 =3D strcmp(evsel__name=
(evsel), "raw_syscalls:sys_exit") =3D=3D 0;
> +                       bool raw_syscalls_sys_exit =3D evsel__name_is(evs=
el, "raw_syscalls:sys_exit");
>
>                         if (raw_syscalls_sys_exit) {
>                                 trace.raw_augmented_syscalls =3D true;
> diff --git a/tools/perf/tests/evsel-roundtrip-name.c b/tools/perf/tests/e=
vsel-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), na=
me)) {
> +                                       if (!evsel__name_is(evsel, name))=
 {
>                                                 pr_debug("%s !=3D %s\n", =
evsel__name(evsel), name);
>                                                 ret =3D TEST_FAIL;
>                                         }
> @@ -71,7 +71,7 @@ static int perf_evsel__name_array_test(const char *cons=
t 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 !=3D %s\n", evsel__name(evse=
l), names[i]);
>                                 ret =3D TEST_FAIL;
>                         }
> diff --git a/tools/perf/tests/parse-events.c b/tools/perf/tests/parse-eve=
nts.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(struc=
t 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(str=
uct 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(str=
uct 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(str=
uct 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(st=
ruct 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_nam=
e(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_nam=
e(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_nam=
e(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_na=
me(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(str=
uct evlist *evlist)
>         TEST_ASSERT_VAL("wrong number of entries", 2 =3D=3D evlist->core.=
nr_entries);
>
>         TEST_ASSERT_VAL("wrong type", PERF_TYPE_BREAKPOINT =3D=3D evsel->=
core.attr.type);
> -       TEST_ASSERT_VAL("wrong name", !strcmp(evsel__name(evsel), "breakp=
oint1"));
> +       TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "breakpoint1"=
));
>
>         evsel =3D evsel__next(evsel);
>
>         TEST_ASSERT_VAL("wrong type", PERF_TYPE_BREAKPOINT =3D=3D evsel->=
core.attr.type);
> -       TEST_ASSERT_VAL("wrong name", !strcmp(evsel__name(evsel), "breakp=
oint2"));
> +       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 =3D=3D evlist->core.=
nr_entries);
>         TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW =3D=3D evsel->core.at=
tr.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=3D2/u" */
>         evsel =3D evsel__next(evsel);
>         TEST_ASSERT_VAL("wrong number of entries", 2 =3D=3D evlist->core.=
nr_entries);
>         TEST_ASSERT_VAL("wrong type", PERF_TYPE_RAW =3D=3D evsel->core.at=
tr.type);
>         TEST_ASSERT_VAL("wrong config", test_config(evsel, 2));
> -       TEST_ASSERT_VAL("wrong name",
> -                       !strcmp(evsel__name(evsel), "cpu/config=3D2/u"));
> +       TEST_ASSERT_VAL("wrong name", evsel__name_is(evsel, "cpu/config=
=3D2/u"));
>
>         return TEST_OK;
>  }
> --
> 2.25.1
>