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 <[email protected]>
Signed-off-by: Namhyung Kim <[email protected]>
---
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;
else
err = TEST_CODE_READING_OK;
--
2.42.0.869.gea05f2083d-goog
On Thu, Nov 2, 2023 at 4:40 PM Namhyung Kim <[email protected]> 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 <[email protected]>
> Signed-off-by: Namhyung Kim <[email protected]>
> ---
> 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
>
On 02/11/2023 23:40, Namhyung Kim 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 <[email protected]>
> Signed-off-by: Namhyung Kim <[email protected]>
> ---
> tools/perf/tests/code-reading.c | 76 ++++++++++-----------------------
> 1 file changed, 23 insertions(+), 53 deletions(-)
>
Tested-by: James Clark <[email protected]>
> 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;
> else
> err = TEST_CODE_READING_OK;