lkml.org 
[lkml]   [2020]   [Mar]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] perf test x86: address multiplexing in rdpmc test
On Fri, Mar 20, 2020 at 05:14:00PM -0700, Ian Rogers wrote:
> Counters may be being used for pinned or other events which inhibit the
> instruction counter in the test from being scheduled - time_enabled > 0
> but time_running == 0. This causes the test to fail with division by 0.
> Add a sleep loop to ensure that the counter is run before computing
> the count.
>
> Signed-off-by: Ian Rogers <irogers@google.com>
> ---
> tools/perf/arch/x86/tests/rdpmc.c | 45 ++++++++++++++++++++++++-------
> 1 file changed, 35 insertions(+), 10 deletions(-)
>
> diff --git a/tools/perf/arch/x86/tests/rdpmc.c b/tools/perf/arch/x86/tests/rdpmc.c
> index 1ea916656a2d..0b0792ae67f7 100644
> --- a/tools/perf/arch/x86/tests/rdpmc.c
> +++ b/tools/perf/arch/x86/tests/rdpmc.c
> @@ -34,19 +34,35 @@ static u64 rdtsc(void)
> return low | ((u64)high) << 32;
> }
>
> -static u64 mmap_read_self(void *addr)
> +static u64 mmap_read_self(void *addr, bool *error)
> {
> struct perf_event_mmap_page *pc = addr;
> - u32 seq, idx, time_mult = 0, time_shift = 0;
> + u32 seq, idx, time_mult = 0, time_shift = 0, sleep_count = 0;
> u64 count, cyc = 0, time_offset = 0, enabled, running, delta;
>
> + *error = false;
> do {
> - seq = pc->lock;
> - barrier();
> -
> - enabled = pc->time_enabled;
> - running = pc->time_running;
> -
> + do {
> + seq = pc->lock;
> + barrier();
> +
> + enabled = pc->time_enabled;
> + running = pc->time_running;
> +
> + if (running == 0) {

This is not in fact the condition the Changelog calls out.

> + /*
> + * Event hasn't run, this may be caused by
> + * multiplexing.
> + */
> + sleep_count++;
> + if (sleep_count > 60) {
> + pr_err("Event failed to run. Are higher priority counters being sampled by a different process?\n");
> + *error = true;
> + return 0;
> + }
> + sleep(1);
> + }
> + } while (running == 0);


Also, I would much prefer this test to be in the caller of this
function, and not deface this function.

I'd prefer this function to stay representative of the outlines in
uapi/linux/perf_event.h and an example of how to actually use it.

> if (enabled != running) {
> cyc = rdtsc();
> time_mult = pc->time_mult;
> @@ -131,13 +147,22 @@ static int __test__rdpmc(void)
>
> for (n = 0; n < 6; n++) {
> u64 stamp, now, delta;
> + bool error;
>
> - stamp = mmap_read_self(addr);
> + stamp = mmap_read_self(addr, &error);
> + if (error) {
> + delta_sum = 0;
> + goto out_close;
> + }
>
> for (i = 0; i < loops; i++)
> tmp++;
>
> - now = mmap_read_self(addr);
> + now = mmap_read_self(addr, &error);
> + if (error) {
> + delta_sum = 0;
> + goto out_close;
> + }
> loops *= 10;
>
> delta = now - stamp;
> --
> 2.25.1.696.g5e7596f4ac-goog
>

\
 
 \ /
  Last update: 2020-03-21 14:41    [W:0.048 / U:1.172 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site