syscalls/perf_event_open03: skip test on slower systems

Some systems (specially with combination of -debug kernel
with KASAN enabled) have trouble completing this test
in specified timeout.

Lowering number of iterations would make the test condition
less accurate as it's based on global counter.

Instead, calculate the rate of iterations system can do in
first 5 seconds and used that to decide whether to continue
to run the test. If the rate is too slow, TCONF after 5
seconds.

Signed-off-by: Jan Stancek <jstancek@redhat.com>
---
 .../perf_event_open/perf_event_open03.c       | 43 ++++++++++++++++++-
 1 file changed, 41 insertions(+), 2 deletions(-)

.needs_cmds = NULL gets rid of compile warning.

Jan Stancek <jstancek@redhat.com> wrote:

Some systems (specially with combination of -debug kernel
> with KASAN enabled) have trouble completing this test
> in specified timeout.
>
> Lowering number of iterations would make the test condition
> less accurate as it's based on global counter.
>
> Instead, calculate the rate of iterations system can do in
> first 5 seconds and used that to decide whether to continue
> to run the test. If the rate is too slow, TCONF after 5
> seconds.
>

Generally, this method looks good, but maybe better to limit this
check_progress() only perform on -debug kernel?  Otherwise,

Reviewed-by: Li Wang <liwang@redhat.com>


======= FYI ==========
I'm seeking a fair way to make a global evaluation of the test
system to reset timeout dynamically for the whole LTP.

My original design thoughts:

  Create the numbers of threads equal to CPUs and bind them to
  the corresponding cpu for running. Use mutex lock to sync up
  each thread launch at the same time to collect the basic data
  for their CPU. Then we can compare the CPU state under the idle or
  busy time to get a relatively stationary _value_ to measure the system
  performance.

But so far the test method is not stable&reliable as expected.

  // do float computing + dirty 10*pagesz memory  in a limited times
  one_unit_of_operation();

  // count the CPU looping numbers with (type = idel, calcu)
  // and call one_unit_opertaion() in 1 sec
  cpu_1sec_looping(int type);

  idlespeed_loops = cpu_1sec_looping(idel);
  calculate_loops = cpu_1sec_looping(calcu);
  ...
  // count the _value_ from all CPU average loops
  ratio = calculate_avg / idealspeed_avg;

Hi!
> Some systems (specially with combination of -debug kernel
> with KASAN enabled) have trouble completing this test
> in specified timeout.
> 
> Lowering number of iterations would make the test condition
> less accurate as it's based on global counter.
> 
> Instead, calculate the rate of iterations system can do in
> first 5 seconds and used that to decide whether to continue
> to run the test. If the rate is too slow, TCONF after 5
> seconds.
> 
> Signed-off-by: Jan Stancek <jstancek@redhat.com>
> ---
>  .../perf_event_open/perf_event_open03.c       | 43 ++++++++++++++++++-
>  1 file changed, 41 insertions(+), 2 deletions(-)
> 
> .needs_cmds = NULL gets rid of compile warning.

Please no workarounds for compiler bugs like this one.

This has been unfortunatelly broken in gcc for at least four releases
now but I do not think that it makes sense to add random field
initializers to most of the tests in the LTP sources.

> diff --git a/testcases/kernel/syscalls/perf_event_open/perf_event_open03.c b/testcases/kernel/syscalls/perf_event_open/perf_event_open03.c
> index dcb70962771c..c7bf123a04b4 100644
> --- a/testcases/kernel/syscalls/perf_event_open/perf_event_open03.c
> +++ b/testcases/kernel/syscalls/perf_event_open/perf_event_open03.c
> @@ -16,13 +16,16 @@
>  
>  #include "config.h"
>  #include "tst_test.h"
> +#include "tst_timer_test.h"
>  #include "lapi/syscalls.h"
>  
>  #include "perf_event_open.h"
>  
>  #define INTEL_PT_PATH "/sys/bus/event_source/devices/intel_pt/type"
>  
> +const int iterations = 12000000;
>  static int fd = -1;
> +static int timeout;
>  
>  static void setup(void)
>  {
> @@ -39,6 +42,38 @@ static void setup(void)
>  
>  	SAFE_FILE_SCANF(INTEL_PT_PATH, "%d", &ev.type);
>  	fd = perf_event_open(&ev, getpid(), -1, -1, 0);
> +
> +	timeout = tst_timeout_remaining();
> +}
> +
> +/*
> + * Check how fast we can do the iterations after 5 seconds of runtime.
> + * If the rate is too small to complete for current timeout then
> + * stop the test.
> + */
> +static void check_progress(int i)
> +{
> +	static float iter_per_ms;
> +	long long elapsed_ms;
> +
> +	if (iter_per_ms)
> +		return;
> +
> +	if (i % 1000 != 0)
> +		return;
> +
> +	tst_timer_stop();
> +	elapsed_ms = tst_timer_elapsed_ms();
> +	if (elapsed_ms > 5000) {
> +		iter_per_ms = (float) i / elapsed_ms;
> +		tst_res(TINFO, "rate: %f iters/ms", iter_per_ms);
> +		tst_res(TINFO, "needed rate for current test timeout: %f iters/ms",
> +			(float) iterations / (timeout * 1000));
> +
> +		if (iter_per_ms * 1000 * (timeout - 1) < iterations)
> +			tst_brk(TCONF, "System too slow to complete"
> +				" test in specified timeout");

String shouldn't be split like that even if they are over 80 chars, at
least if I remmeber LKML coding style correctly, the reason is that
splitting them like this breaks git grep for the output messages.

> +	}
>  }
>  
>  static void run(void)
> @@ -47,10 +82,13 @@ static void run(void)
>  	int i;
>  
>  	diff = SAFE_READ_MEMINFO("MemAvailable:");
> +	tst_timer_start(CLOCK_MONOTONIC);
>  
>  	/* leak about 100MB of RAM */
> -	for (i = 0; i < 12000000; i++)
> +	for (i = 0; i < iterations; i++) {
>  		ioctl(fd, PERF_EVENT_IOC_SET_FILTER, "filter,0/0@abcd");
> +		check_progress(i);
> +	}

Generally looks good. I guess that I will have to consider turning this
into a generic functionality when I return to the runtime patchset, but
for now this should be good to go.

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

Hi!
> Generally, this method looks good, but maybe better to limit this
> check_progress() only perform on -debug kernel?  Otherwise,

I guess that it may be pretty useful on embedded systems as well. Having
TCONF message with "system too slow" rather than a timeout sounds like
an improvement to me.

> Reviewed-by: Li Wang <liwang@redhat.com>
> 
> 
> ======= FYI ==========
> I'm seeking a fair way to make a global evaluation of the test
> system to reset timeout dynamically for the whole LTP.
> 
> My original design thoughts:
> 
>   Create the numbers of threads equal to CPUs and bind them to
>   the corresponding cpu for running. Use mutex lock to sync up
>   each thread launch at the same time to collect the basic data
>   for their CPU. Then we can compare the CPU state under the idle or
>   busy time to get a relatively stationary _value_ to measure the system
>   performance.
> 
> But so far the test method is not stable&reliable as expected.
> 
>   // do float computing + dirty 10*pagesz memory  in a limited times
>   one_unit_of_operation();
> 
>   // count the CPU looping numbers with (type = idel, calcu)
>   // and call one_unit_opertaion() in 1 sec
>   cpu_1sec_looping(int type);
> 
>   idlespeed_loops = cpu_1sec_looping(idel);
>   calculate_loops = cpu_1sec_looping(calcu);
>   ...
>   // count the _value_ from all CPU average loops
>   ratio = calculate_avg / idealspeed_avg;

I'm not sure how useful this would be, I guess that the speed of
different syscalls will differ quite a bit on different kernel versions.
Maybe the whole system is too complex and cannot be described by
something as simple as this. But I guess that we will not know unless we
try.

On Wed, Feb 23, 2022 at 10:09 PM Cyril Hrubis <chrubis@suse.cz> wrote:

> Hi!
> > Generally, this method looks good, but maybe better to limit this
> > check_progress() only perform on -debug kernel?  Otherwise,
>
> I guess that it may be pretty useful on embedded systems as well. Having
> TCONF message with "system too slow" rather than a timeout sounds like
> an improvement to me.
>

Yes, sounds reasonable.



>
> > Reviewed-by: Li Wang <liwang@redhat.com>
> >
> >
> > ======= FYI ==========
> > I'm seeking a fair way to make a global evaluation of the test
> > system to reset timeout dynamically for the whole LTP.
> >
> > My original design thoughts:
> >
> >   Create the numbers of threads equal to CPUs and bind them to
> >   the corresponding cpu for running. Use mutex lock to sync up
> >   each thread launch at the same time to collect the basic data
> >   for their CPU. Then we can compare the CPU state under the idle or
> >   busy time to get a relatively stationary _value_ to measure the system
> >   performance.
> >
> > But so far the test method is not stable&reliable as expected.
> >
> >   // do float computing + dirty 10*pagesz memory  in a limited times
> >   one_unit_of_operation();
> >
> >   // count the CPU looping numbers with (type = idel, calcu)
> >   // and call one_unit_opertaion() in 1 sec
> >   cpu_1sec_looping(int type);
> >
> >   idlespeed_loops = cpu_1sec_looping(idel);
> >   calculate_loops = cpu_1sec_looping(calcu);
> >   ...
> >   // count the _value_ from all CPU average loops
> >   ratio = calculate_avg / idealspeed_avg;
>
> I'm not sure how useful this would be, I guess that the speed of
> different syscalls will differ quite a bit on different kernel versions.
> Maybe the whole system is too complex and cannot be described by
> something as simple as this. But I guess that we will not know unless we
> try.
>

That's right, after try with many times locally I realized that we probably
need a general-purpose function or macro that can pass with a kind of
syscall to get the performance value of it. But that needs more
consideration
and practice, so I add this to my long-term TODO list.

> Generally looks good. I guess that I will have to consider turning this
> into a generic functionality when I return to the runtime patchset, but
> for now this should be good to go.
>
> Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

Pushed with compiler workaround removed and string kept on single line.