Skip to contentSkip to conversation

syscalls: Add timer measurement library

RFC PATCH New Cyril Hrubis 2 participants 8 messages May 26, 2017 – Jun 1, 2017 Library code

lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/1 syscalls: Add timer measurement library

This commit adds a timer measurement library, mostly based on changes done to the pselect01.c test and changes all tests that measure timer precision to use it.

The timer testcases that measure timeouts now just define sampling function and optional setup and cleanup. The rest of the functionality is implemented in the lib/tst_timer_test.c library. This change not only removes fair amount of duplicated code but also allows us to tune thresholds and define testcases in a single place for all testcases.

The timer measurement library also supports for passing sleep time and number of iterations as a command-line parameters, can print nifty frequency plot into the terminal, as well as save test measurements into a text file.

---
 include/tst_timer.h                                |  26 ++
 include/tst_timer_test.h                           |  74 ++++
 lib/tst_timer_test.c                               | 434 +++++++++++++++++++++
 runtest/syscalls                                   |   1 +
 testcases/kernel/syscalls/.gitignore               |   1 +
 .../syscalls/clock_nanosleep/clock_nanosleep01.c   |  67 +---
 .../syscalls/clock_nanosleep/clock_nanosleep02.c   |  49 +++
 .../kernel/syscalls/epoll_wait/epoll_wait02.c      | 125 ++----
 testcases/kernel/syscalls/futex/futex_wait05.c     |  61 +--
 testcases/kernel/syscalls/nanosleep/nanosleep01.c  |  56 +--
 testcases/kernel/syscalls/poll/poll02.c            | 104 ++---
 testcases/kernel/syscalls/pselect/pselect01.c      | 161 ++------
 testcases/kernel/syscalls/select/select04.c        | 103 ++---
 13 files changed, 746 insertions(+), 516 deletions(-)
 create mode 100644 include/tst_timer_test.h
 create mode 100644 lib/tst_timer_test.c
 create mode 100644 testcases/kernel/syscalls/clock_nanosleep/clock_nanosleep02.c
include/tst_timer.h +26 -0
index f0a10bd45..0448f4428 100644
@@ -77,6 +77,32 @@ static inline struct timeval tst_us_to_timeval(long long us)
 }
 
 /*
+ * Converts ms to struct timespec
+ */
+static inline struct timespec tst_ms_to_timespec(long long us)
+{
+	struct timespec ret;
+
+	ret.tv_sec = us / 1000;
+	ret.tv_nsec = (us % 1000) * 1000000;
+
+	return ret;
+}
+
+/*
+ * Converts us to struct timespec
+ */
+static inline struct timespec tst_us_to_timespec(long long us)
+{
+	struct timespec ret;
+
+	ret.tv_sec = us / 1000000;
+	ret.tv_nsec = (us % 1000000) * 1000;
+
+	return ret;
+}
+
+/*
  * Comparsions
  */
 static inline int tst_timespec_lt(struct timespec t1, struct timespec t2)
include/tst_timer_test.h new file +74 -0

74 lines not on this page · raw message

lib/tst_timer_test.c new file +434 -0

434 lines not on this page · raw message

runtest/syscalls +1 -0
index 06ce51e92..12a38eeab 100644
@@ -74,6 +74,7 @@ chroot04 chroot04
 
 clock_getres01 clock_getres01
 clock_nanosleep01 clock_nanosleep01
+clock_nanosleep02 clock_nanosleep02
 clock_nanosleep2_01 clock_nanosleep2_01
 
 clone01 clone01
testcases/kernel/syscalls/.gitignore +1 -0
index 497e58ef5..46a29e99e 100644
@@ -53,6 +53,7 @@
 /chroot/chroot04
 /clock_getres/clock_getres01
 /clock_nanosleep/clock_nanosleep01
+/clock_nanosleep/clock_nanosleep02
 /clock_nanosleep2/clock_nanosleep2_01
 /clone/clone01
 /clone/clone02
testcases/kernel/syscalls/clock_nanosleep/clock_nanosleep01.c +17 -50

107 lines not on this page · raw message

testcases/kernel/syscalls/clock_nanosleep/clock_nanosleep02.c new file +49 -0

49 lines not on this page · raw message

testcases/kernel/syscalls/epoll_wait/epoll_wait02.c +32 -93

166 lines not on this page · raw message

testcases/kernel/syscalls/futex/futex_wait05.c +16 -45

85 lines not on this page · raw message

testcases/kernel/syscalls/nanosleep/nanosleep01.c +17 -39

80 lines not on this page · raw message

testcases/kernel/syscalls/poll/poll02.c +26 -78

136 lines not on this page · raw message

testcases/kernel/syscalls/pselect/pselect01.c +29 -132

181 lines not on this page · raw message

testcases/kernel/syscalls/select/select04.c +24 -79

133 lines not on this page · raw message

Signature · 1 line
2.13.0
#1 lore raw

RFC PATCH 1/1 syscalls: Add timer measurement library

----- Original Message -----

Quoted 45 lines

This commit adds a timer measurement library, mostly based on changes done to the pselect01.c test and changes all tests that measure timer precision to use it.

The timer testcases that measure timeouts now just define sampling function and
optional setup and cleanup. The rest of the functionality is implemented in the
lib/tst_timer_test.c library. This change not only removes fair amount of duplicated code but also allows us to tune thresholds and define testcases in a
single place for all testcases.

The timer measurement library also supports for passing sleep time and number of iterations as a command-line parameters, can print nifty frequency plot into the terminal, as well as save test measurements into a text file.

---
 include/tst_timer.h                                |  26 ++
 include/tst_timer_test.h                           |  74 ++++
 lib/tst_timer_test.c                               | 434
 +++++++++++++++++++++
 runtest/syscalls                                   |   1 +
 testcases/kernel/syscalls/.gitignore               |   1 +
 .../syscalls/clock_nanosleep/clock_nanosleep01.c   |  67 +---
 .../syscalls/clock_nanosleep/clock_nanosleep02.c   |  49 +++
 .../kernel/syscalls/epoll_wait/epoll_wait02.c      | 125 ++----
 testcases/kernel/syscalls/futex/futex_wait05.c     |  61 +--
 testcases/kernel/syscalls/nanosleep/nanosleep01.c  |  56 +--
 testcases/kernel/syscalls/poll/poll02.c            | 104 ++---
 testcases/kernel/syscalls/pselect/pselect01.c      | 161 ++------
 testcases/kernel/syscalls/select/select04.c        | 103 ++---
 13 files changed, 746 insertions(+), 516 deletions(-)
 create mode 100644 include/tst_timer_test.h
 create mode 100644 lib/tst_timer_test.c
 create mode 100644

testcases/kernel/syscalls/clock_nanosleep/clock_nanosleep02.c

include/tst_timer.h +0 -0
index f0a10bd45..0448f4428 100644

include/tst_timer.h static inline struct timeval tst_us_to_timeval(long long · line 83

Quoted 7 lines
include/tst_timer.h +3 -0
@@ -77,6 +77,32 @@ static inline struct timeval tst_us_to_timeval(long long
 us)
 }
 
 /*
+ * Converts ms to struct timespec
+ */
+static inline struct timespec tst_ms_to_timespec(long long us)

Hi,

us -> ms

Quoted 9 lines
include/tst_timer.h +9 -0
+{
+	struct timespec ret;
+
+	ret.tv_sec = us / 1000;
+	ret.tv_nsec = (us % 1000) * 1000000;
+
+	return ret;
+}
+

<snip>

include/tst_timer.h

Quoted 7 lines
include/tst_timer.h +7 -0
+
+struct tst_timer_test {
+	const char *scall;
+	int (*sample)(int clk_id, long long usec);
+	void (*setup)(void);
+	void (*cleanup)(void);
+};

I'd rather keep tst_test struct and expose some new function, that would do all of this.

void test_all()

{
    tst_timer_test("select()", test_sample_function);
}

void tst_timer_test(fn_name, test_sample_function) {
timer_parse_options();
timer_setup();

  for n ... {
     do_timer_test(timer_tcases[n].usec, timer_tcases[n].samples, test_sample_function);

}
timer_cleanup();
}

What I'm afraid of is that we end up mirror-ing lot of functionality in tst_test struct: needsroot, tmpdir, kernelversion, extra parameter

Quoted 7 lines
include/tst_timer.h +7 -0
+void do_timer_test(long long usec, unsigned int nsamples)
+{
+	long long trunc_mean, median;
+	unsigned int discard = compute_discard(nsamples);
+	unsigned int keep_samples = nsamples - discard;
+	long long threshold = compute_threshold(usec, keep_samples);
+	unsigned int i;

include/tst_timer.h

Quoted 10 lines
include/tst_timer.h +10 -0
+	int failed = 0;
+
+	tst_res(TINFO,
+		"%s sleeping for %llius %u iterations, threshold %.2fus",
+		timer_test->scall, usec, nsamples,
+		1.00 * threshold / (keep_samples));
+
+	cur_sample = 0;
+	for (i = 0; i < nsamples; i++) {
+		if (timer_test->sample(CLOCK_REALTIME, usec)) {

Since we use resolution of monotonic clock, should we also measure with it here?

include/tst_timer.h

Quoted 10 lines
include/tst_timer.h +10 -0
+			tst_res(TINFO, "sampling function failed, exitting");
+			return;
+		}
+	}
+
+	qsort(samples, nsamples, sizeof(samples[0]), cmp);
+
+	write_to_file();
+
+	for (i = 0; samples[i] > 10 * usec; i++) {

This could also use a range check for length of samples array, just in case all samples are outliners.

Regards,
Jan

#2 lore raw

RFC PATCH 1/1 syscalls: Add timer measurement library

Hi!

Quoted 20 lines

include/tst_timer_test.h line 60

Quoted 7 lines
include/tst_timer_test.h new file +7 -0
+
+struct tst_timer_test {
+	const char *scall;
+	int (*sample)(int clk_id, long long usec);
+	void (*setup)(void);
+	void (*cleanup)(void);
+};

I'd rather keep tst_test struct and expose some new function, that would do all of this.

void test_all()

{
    tst_timer_test("select()", test_sample_function);
}

void tst_timer_test(fn_name, test_sample_function) {
timer_parse_options();
timer_setup();

Quoted 8 lines
  for n ... {
     do_timer_test(timer_tcases[n].usec, timer_tcases[n].samples, test_sample_function);

}
timer_cleanup();
}

What I'm afraid of is that we end up mirror-ing lot of functionality in tst_test struct: needsroot, tmpdir, kernelversion, extra parameter

Hmm, we can also fill in the standard tst_test structure in the test then "override" some of the fields in the timer test library.

I.e. test defines tst_test structure and the sampling function, timer library main() stores and replaces setup & cleanup, sets the test function then calls the tst_run_tcases() function. Does that sound better?

Quoted 12 lines
Quoted 12 lines
lib/tst_timer_test.c new file +12 -0
+void do_timer_test(long long usec, unsigned int nsamples)
+{
+	long long trunc_mean, median;
+	unsigned int discard = compute_discard(nsamples);
+	unsigned int keep_samples = nsamples - discard;
+	long long threshold = compute_threshold(usec, keep_samples);
+	unsigned int i;
+	int failed = 0;
+
+	tst_res(TINFO,
+		"%s sleeping for %llius %u iterations, threshold %.2fus",
+		timer_test->scall, usec, nsamples,
Quoted 8 lines

lib/tst_timer_test.c line 266

Quoted 5 lines
lib/tst_timer_test.c new file +5 -0
+		1.00 * threshold / (keep_samples));
+
+	cur_sample = 0;
+	for (i = 0; i < nsamples; i++) {
+		if (timer_test->sample(CLOCK_REALTIME, usec)) {

Since we use resolution of monotonic clock, should we also measure with it here?

Ah, my bad, I've put wrong clock ID here by accident. Also I've been thinking if we should use CLOCK_MONOTONIC_RAW if it's available then fallback to CLOCK_MONOTONIC. Since CLOCK_MONOTONIC may be subject to ntpd adjustements. Also can you try with CLOCK_MONOTONIC_RAW on moonshot to see if that makes any difference?

Quoted 5 lines
Quoted 5 lines
lib/tst_timer_test.c new file +5 -0
+			tst_res(TINFO, "sampling function failed, exitting");
+			return;
+		}
+	}
+
Quoted 8 lines

lib/tst_timer_test.c line 276

Quoted 5 lines
lib/tst_timer_test.c new file +5 -0
+	qsort(samples, nsamples, sizeof(samples[0]), cmp);
+
+	write_to_file();
+
+	for (i = 0; samples[i] > 10 * usec; i++) {

This could also use a range check for length of samples array, just in case all samples are outliners.

Good catch.

Signature · 2 lines
Cyril Hrubis
[email protected]
#3 lore raw

RFC PATCH 1/1 syscalls: Add timer measurement library

----- Original Message -----

Quoted 31 lines

Hi!

Quoted 23 lines

include/tst_timer_test.h line 60

Quoted 7 lines
include/tst_timer_test.h new file +7 -0
+
+struct tst_timer_test {
+	const char *scall;
+	int (*sample)(int clk_id, long long usec);
+	void (*setup)(void);
+	void (*cleanup)(void);
+};

I'd rather keep tst_test struct and expose some new function, that would do all of this.

void test_all()

{
    tst_timer_test("select()", test_sample_function);
}

void tst_timer_test(fn_name, test_sample_function) {
timer_parse_options();
timer_setup();

  for n ... {
     do_timer_test(timer_tcases[n].usec, timer_tcases[n].samples,
     test_sample_function);
Quoted 6 lines

}
timer_cleanup();
}

What I'm afraid of is that we end up mirror-ing lot of functionality in tst_test struct: needsroot, tmpdir, kernelversion, extra parameter

Quoted 7 lines

Hmm, we can also fill in the standard tst_test structure in the test then "override" some of the fields in the timer test library.

I.e. test defines tst_test structure and the sampling function, timer library main() stores and replaces setup & cleanup, sets the test function then calls the tst_run_tcases() function. Does that sound better?

Yes. This would mean we extend tst_test struct and add sampling function. As long as rest of fields continue to work (with some exceptions [1]), it sounds good.

I see you still want to keep timer library main(), but I'm not sure how you are going to select it instead of tst_test main.

[1] obvious exception is that we can't allow test/test_all and sampling function at the same time.

#4 lore raw

RFC PATCH 1/1 syscalls: Add timer measurement library

Hi!

Quoted 11 lines
Quoted 7 lines

Hmm, we can also fill in the standard tst_test structure in the test then "override" some of the fields in the timer test library.

I.e. test defines tst_test structure and the sampling function, timer library main() stores and replaces setup & cleanup, sets the test function then calls the tst_run_tcases() function. Does that sound better?

Yes. This would mean we extend tst_test struct and add sampling function. As long as rest of fields continue to work (with some exceptions [1]), it sounds good.

We can setup the test/sample functions as an union. As we can then distinguish between them based on .tcnt being non-zero in a case of standard test and expect sampling function to be set in a case of the timer test. However this makes the whole interface less type safe and also much harder to extend. I was thinking of adding support for NULL terminated array of test funcitons as well since we have some testcases that use the test() function as dispatcher with a long switch() statement...

So maybe we should just add a few more function pointers there.

Quoted 2 lines

I see you still want to keep timer library main(), but I'm not sure how you are going to select it instead of tst_test main.

Only the tst_timer_test.h should be included in the timer testcases which includes tst_test.h after defining TST_NO_DEFAULT_MAIN, then we can define timer specific main that saves cleanup/setup and replaces these with the timer test cleanup/setup that calls the test specifc cleanup/setup functions. The timer library main also has to set up the test function and test options in the tst_test structure since I want to avoid setting these in each of the testcases to avoid copy & paste errors.

Signature · 2 lines
Cyril Hrubis
[email protected]
#5 lore raw

RFC PATCH 1/1 syscalls: Add timer measurement library

Hi!

Quoted 21 lines
Quoted 11 lines
Quoted 7 lines

Hmm, we can also fill in the standard tst_test structure in the test then "override" some of the fields in the timer test library.

I.e. test defines tst_test structure and the sampling function, timer library main() stores and replaces setup & cleanup, sets the test function then calls the tst_run_tcases() function. Does that sound better?

Yes. This would mean we extend tst_test struct and add sampling function. As long as rest of fields continue to work (with some exceptions [1]), it sounds good.

We can setup the test/sample functions as an union. As we can then distinguish between them based on .tcnt being non-zero in a case of standard test and expect sampling function to be set in a case of the timer test. However this makes the whole interface less type safe and also much harder to extend. I was thinking of adding support for NULL terminated array of test funcitons as well since we have some testcases that use the test() function as dispatcher with a long switch() statement...

Quoted 1 line

So maybe we should just add a few more function pointers there.

Or we may as well avoid adding sampling function to the test structure and pass it explicitly in the timer library main().

int main(int argc, char *argv[])

{
	tst_run_timer_testcases(argc, argv, test, sample_fn, scall);
}

We do that with the test structure anyway so this would be a small logical extension. And we may as well pass the syscall name in tid field of the test stucture...

Signature · 2 lines
Cyril Hrubis
[email protected]
#6 lore raw

RFC PATCH 1/1 syscalls: Add timer measurement library

In reply to Cyril Hrubis

----- Original Message -----

Quoted 9 lines

Hi!

Quoted 8 lines
Quoted 7 lines

Hmm, we can also fill in the standard tst_test structure in the test then "override" some of the fields in the timer test library.

I.e. test defines tst_test structure and the sampling function, timer library main() stores and replaces setup & cleanup, sets the test function then calls the tst_run_tcases() function. Does that sound better?

Quoted 8 lines
Quoted 3 lines

Yes. This would mean we extend tst_test struct and add sampling function. As long as rest of fields continue to work (with some exceptions [1]), it sounds good.

We can setup the test/sample functions as an union. As we can then distinguish between them based on .tcnt being non-zero in a case of standard test and expect sampling function to be set in a case of the timer test.

tcnt can be also zero if you use test_all()

Quoted 2 lines

However this makes the whole interface less type safe and also much harder to extend.

Agreed, I'd avoid union.

Quoted 10 lines

I was thinking of adding support for NULL
terminated array of test funcitons as well since we have some testcases that use the test() function as dispatcher with a long switch() statement...

So maybe we should just add a few more function pointers there.

Quoted 2 lines

I see you still want to keep timer library main(), but I'm not sure how you are going to select it instead of tst_test main.

Quoted 8 lines

Only the tst_timer_test.h should be included in the timer testcases which includes tst_test.h after defining TST_NO_DEFAULT_MAIN, then we can define timer specific main that saves cleanup/setup and replaces these with the timer test cleanup/setup that calls the test specifc cleanup/setup functions. The timer library main also has to set up the test function and test options in the tst_test structure since I want to avoid setting these in each of the testcases to avoid copy & paste errors.

What if we modified lib/tst_test.c to make calls to timerlib where needed? User would just include tst_test.h as before, but instead of test_all he sets sampling_func in tst_test struct.

lib/tst_test.c +27 -1
index 794aa8555b3a..a0ddb674e2b9 100644
@@ -669,6 +669,9 @@ static void do_setup(int argc, char *argv[])
                tst_test->format_device = 1;
        }
 
+       if (tst_test->sampling_func)
+               timerlib_add_opts(tst_test);
+
        parse_opts(argc, argv);
 
        setup_ipc();
@@ -746,7 +749,23 @@ static void run_tests(void)
        unsigned int i;
        struct results saved_results;
 
-       if (!tst_test->test) {
+       if (tst_test->sampling_func) {
+
+               timerlib_runtest(tst_test->sampling_func);
+
+               if (getpid() != main_pid) {
+                       exit(0);
+               }
+
+               tst_reap_children();
+
+               if (results_equal(&saved_results, results))
+                       tst_brk(TBROK, "Test haven't reported results!");
+               return;
+
+       }
+
+       if (tst_test->test_all) {
                saved_results = *results;
                tst_test->test_all();
@@ -761,6 +780,7 @@ static void run_tests(void)
                return;
        }
 
+       if (tst_test->test) {
        for (i = 0; i < tst_test->tcnt; i++) {
                saved_results = *results;
                tst_test->test(i);
@@ -774,6 +794,7 @@ static void run_tests(void)
                if (results_equal(&saved_results, results))
                        tst_brk(TBROK, "Test %i haven't reported results!", i);
        }
+       }
 }
 
 static unsigned long long get_time_ms(void)
@@ -791,6 +812,8 @@ static void testrun(void)
        unsigned long long stop_time = 0;
        int cont = 1;
 
+       if (tst_test->sampling_func)
+               timerlib_setup();
        do_test_setup();
 
        if (duration > 0)
@@ -816,6 +839,9 @@ static void testrun(void)
        }
 
        do_test_cleanup();
+
+       if (tst_test->sampling_func)
+               timerlib_cleanup();
        exit(0);
 }
 
#7 lore raw

RFC PATCH 1/1 syscalls: Add timer measurement library

Hi!

Quoted 3 lines

What if we modified lib/tst_test.c to make calls to timerlib where needed? User would just include tst_test.h as before, but instead of test_all he sets sampling_func in tst_test struct.

I was trying to avoid that so that we do not overcomplicate the already complex tst_test.c but the changes does not look that scary at all. I will try to prepare v2 patch.

Signature · 2 lines
Cyril Hrubis
[email protected]