| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAP-5=fUuDB6W8yUaWu_7SVK5CGBuRLHh8amERQ2dqka_eOX6XQ@mail.gmail.com>
Date: Thu, 2 Nov 2023 16:55:50 -0700
From: Ian Rogers <irogers@...gle.com>
To: Namhyung Kim <namhyung@...nel.org>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>,
Jiri Olsa <jolsa@...nel.org>,
Adrian Hunter <adrian.hunter@...el.com>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...nel.org>,
LKML <linux-kernel@...r.kernel.org>,
linux-perf-users@...r.kernel.org,
Thomas Richter <tmricht@...ux.ibm.com>
Subject: Re: [PATCH] perf test: Simplify object code reading test
On Thu, Nov 2, 2023 at 4:40 PM Namhyung Kim <namhyung@...nel.org> wrote:
>
> It tries cycles (or cpu-clock on s390) event with exclude_kernel bit to
> open. But other arch on a VM can fail with the hardware event and need
> to fallback to the software event in the same way.
>
> So let's get rid of the cpuid check and use generic fallback mechanism
> using an array of event candidates. Now event in the odd index excludes
> the kernel so use that for the return value.
>
> Cc: Thomas Richter <tmricht@...ux.ibm.com>
> Signed-off-by: Namhyung Kim <namhyung@...nel.org>
> ---
> tools/perf/tests/code-reading.c | 76 ++++++++++-----------------------
> 1 file changed, 23 insertions(+), 53 deletions(-)
>
> diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
> index 3af81012014e..047ba297c6fa 100644
> --- a/tools/perf/tests/code-reading.c
> +++ b/tools/perf/tests/code-reading.c
> @@ -511,38 +511,6 @@ static void fs_something(void)
> }
> }
>
> -#ifdef __s390x__
> -#include "header.h" // for get_cpuid()
> -#endif
> -
> -static const char *do_determine_event(bool excl_kernel)
> -{
> - const char *event = excl_kernel ? "cycles:u" : "cycles";
> -
> -#ifdef __s390x__
> - char cpuid[128], model[16], model_c[16], cpum_cf_v[16];
> - unsigned int family;
> - int ret, cpum_cf_a;
> -
> - if (get_cpuid(cpuid, sizeof(cpuid)))
> - goto out_clocks;
> - ret = sscanf(cpuid, "%*[^,],%u,%[^,],%[^,],%[^,],%x", &family, model_c,
> - model, cpum_cf_v, &cpum_cf_a);
> - if (ret != 5) /* Not available */
> - goto out_clocks;
> - if (excl_kernel && (cpum_cf_a & 4))
> - return event;
> - if (!excl_kernel && (cpum_cf_a & 2))
> - return event;
> -
> - /* Fall through: missing authorization */
> -out_clocks:
> - event = excl_kernel ? "cpu-clock:u" : "cpu-clock";
> -
> -#endif
> - return event;
> -}
> -
> static void do_something(void)
> {
> fs_something();
> @@ -583,8 +551,10 @@ static int do_test_code_reading(bool try_kcore)
> int err = -1, ret;
> pid_t pid;
> struct map *map;
> - bool have_vmlinux, have_kcore, excl_kernel = false;
> + bool have_vmlinux, have_kcore;
> struct dso *dso;
> + const char *events[] = { "cycles", "cycles:u", "cpu-clock", "cpu-clock:u", NULL };
> + int evidx = 0;
>
> pid = getpid();
>
> @@ -618,7 +588,7 @@ static int do_test_code_reading(bool try_kcore)
>
> /* No point getting kernel events if there is no kernel object */
> if (!have_vmlinux && !have_kcore)
> - excl_kernel = true;
> + evidx++;
>
> threads = thread_map__new_by_tid(pid);
> if (!threads) {
> @@ -646,7 +616,7 @@ static int do_test_code_reading(bool try_kcore)
> goto out_put;
> }
>
> - while (1) {
> + while (events[evidx]) {
> const char *str;
>
> evlist = evlist__new();
> @@ -657,7 +627,7 @@ static int do_test_code_reading(bool try_kcore)
>
> perf_evlist__set_maps(&evlist->core, cpus, threads);
>
> - str = do_determine_event(excl_kernel);
> + str = events[evidx];
> pr_debug("Parsing event '%s'\n", str);
> ret = parse_event(evlist, str);
> if (ret < 0) {
> @@ -675,32 +645,32 @@ static int do_test_code_reading(bool try_kcore)
>
> ret = evlist__open(evlist);
> if (ret < 0) {
> - if (!excl_kernel) {
> - excl_kernel = true;
> - /*
> - * Both cpus and threads are now owned by evlist
> - * and will be freed by following perf_evlist__set_maps
> - * call. Getting reference to keep them alive.
> - */
> - perf_cpu_map__get(cpus);
> - perf_thread_map__get(threads);
> - perf_evlist__set_maps(&evlist->core, NULL, NULL);
> - evlist__delete(evlist);
> - evlist = NULL;
> - continue;
> - }
> + evidx++;
>
> - if (verbose > 0) {
> + if (events[evidx] == NULL && verbose > 0) {
> char errbuf[512];
> evlist__strerror_open(evlist, errno, errbuf, sizeof(errbuf));
> pr_debug("perf_evlist__open() failed!\n%s\n", errbuf);
> }
>
> - goto out_put;
> + /*
> + * Both cpus and threads are now owned by evlist
> + * and will be freed by following perf_evlist__set_maps
> + * call. Getting reference to keep them alive.
> + */
> + perf_cpu_map__get(cpus);
> + perf_thread_map__get(threads);
> + perf_evlist__set_maps(&evlist->core, NULL, NULL);
> + evlist__delete(evlist);
> + evlist = NULL;
> + continue;
> }
> break;
> }
>
> + if (events[evidx] == NULL)
> + goto out_put;
> +
> ret = evlist__mmap(evlist, UINT_MAX);
> if (ret < 0) {
> pr_debug("evlist__mmap failed\n");
> @@ -721,7 +691,7 @@ static int do_test_code_reading(bool try_kcore)
> err = TEST_CODE_READING_NO_KERNEL_OBJ;
> else if (!have_vmlinux && !try_kcore)
> err = TEST_CODE_READING_NO_VMLINUX;
> - else if (excl_kernel)
> + else if (evidx % 2)
> err = TEST_CODE_READING_NO_ACCESS;
Perhaps it would be more intention revealing to do:
if (strstr(events[evidx], ":u"))
rather than the "evidx % 2".
Thanks,
Ian
> else
> err = TEST_CODE_READING_OK;
> --
> 2.42.0.869.gea05f2083d-goog
>
Powered by blists - more mailing lists