test: drop 'alarm_autotest' and 'cycles_autotest' from test suite
PATCH 1/1 test: drop 'alarm_autotest' and 'cycles_autotest' from test suite
The tests 'alarm_autotest' and 'cycles_autotest' rely on the underlying system having very accurate and precise timing. On systems where the timing isn't as rigid, or the load is particularly high, these tests are unreliable since the wake latency from the scheduler can be high enough to miss the timing window.
Remove these tests from the test suites. Maybe it's useful for these tests to be present as a diagnostics tool, but for normal unit testing, they don't provide much value. They have falsely flagged patches as FAIL on various infrastructures.
--- app/test/meson.build | 2 -- 1 file changed, 2 deletions(-)
app/test/meson.build +0 -2
@@ -185,14 +185,12 @@ test_deps = [ # to indicate whether it can run in no-huge mode. fast_tests = [ ['acl_autotest', true], - ['alarm_autotest', false], ['atomic_autotest', false], ['bitops_autotest', true], ['byteorder_autotest', true], ['cmdline_autotest', true], ['common_autotest', true], ['cpuflags_autotest', true], - ['cycles_autotest', true], ['debug_autotest', true], ['eal_flags_c_opt_autotest', false], ['eal_flags_main_opt_autotest', false],
Signature · 1 line
2.31.1
25/05/2021 02:16, Aaron Conole:
Quoted 10 lines
The tests '
alarm_autotest' and 'cycles_autotest' rely on the underlying system having very accurate and precise timing. On systems where the timing isn't as rigid, or the load is particularly high, these tests are unreliable since the wake latency from the scheduler can be high enough to miss the timing window.Remove these tests from the test suites. Maybe it's useful for these tests to be present as a diagnostics tool, but for normal unit testing, they don't provide much value. They have falsely flagged patches as FAIL on various infrastructures.
Are sure of the value keeping the source code?
Quoted 7 lines · Thomas Monjalon
25/05/2021 02:16, Aaron Conole:
Quoted 6 lines
The tests '
alarm_autotest' and 'cycles_autotest' rely on the underlying system having very accurate and precise timing. On systems where the timing isn't as rigid, or the load is particularly high, these tests are unreliable since the wake latency from the scheduler can be high enough to miss the timing window.
Quoted 6 lines
Quoted 4 lines
Remove these tests from the test suites. Maybe it's useful for these tests to be present as a diagnostics tool, but for normal unit testing, they don't provide much value. They have falsely flagged patches as FAIL on various infrastructures.
Are sure of the value keeping the source code?
Okay, I think it's fine to remove it as well.
I will submit v2
PATCH v2 1/1 test: remove strict timing requirements from alarm and cycles tests
In reply to Aaron ConoleThe tests 'alarm_autotest' and 'cycles_autotest' rely on the underlying system having very accurate and precise timing. On systems where the timing isn't as rigid, or the load is particularly high, these tests are unreliable since the wake latency from the scheduler can be high enough to miss the timing window.
Remove the timing related tests from the test suites. These tests now ensure the add/remove callback infrastructure unit tests, but drop the waits and reliance on system timing and load.
This avoids FAIL on various testing infrastructures.
---
v2: Drop the timing requirements, but keep the API calls
app/test/test_alarm.c | 165 ----------------------------------------- app/test/test_cycles.c | 78 ------------------- 2 files changed, 243 deletions(-)
app/test/test_alarm.c +0 -165
@@ -28,175 +28,13 @@ test_alarm_callback(void *cb_arg)printf ("Callback setting flag - OK. [cb_arg = %p]\n" , cb_arg); } -static rte_atomic32_t cb_count; - -static void -test_multi_cb (void *arg) -{ -rte_atomic32_inc (&cb_count); -printf ("In %s - arg = %p\n" , __func__, arg); -} - -static volatile int recursive_error =0 ; - -static void -test_remove_in_callback (void *arg) -{ -printf ("In %s - arg = %p\n" , __func__, arg); -if (rte_eal_alarm_cancel (test_remove_in_callback, arg) || -rte_eal_alarm_cancel (test_remove_in_callback, (void *)-1 )) { -printf ("Error - cancelling callback from within function succeeded!\n" ); - recursive_error =1 ; - } - flag = (int )((uintptr_t )arg); -} - -static volatile int flag_2; - -static void -test_remove_in_callback_2 (void *arg) -{ -if (rte_eal_alarm_cancel (test_remove_in_callback_2, arg) ||rte_eal_alarm_cancel (test_remove_in_callback_2, (void *)-1 )) { -printf ("Error - cancelling callback of test_remove_in_callback_2\n" ); -return ; - } - flag_2 =1 ; -} - -static int -test_multi_alarms (void ) -{ -int rm_count =0 ; -int count =0 ; - cb_count.cnt =0 ; - -printf ("Expect 6 callbacks in order...\n" ); -/* add two alarms in order */ -rte_eal_alarm_set (10 *US_PER_MS , test_multi_cb, (void *)1 ); -rte_eal_alarm_set (20 *US_PER_MS , test_multi_cb, (void *)2 ); - -/* now add in reverse order */ -rte_eal_alarm_set (60 *US_PER_MS , test_multi_cb, (void *)6 ); -rte_eal_alarm_set (50 *US_PER_MS , test_multi_cb, (void *)5 ); -rte_eal_alarm_set (40 *US_PER_MS , test_multi_cb, (void *)4 ); -rte_eal_alarm_set (30 *US_PER_MS , test_multi_cb, (void *)3 ); - -/* wait for expiry */ -rte_delay_ms (65 ); -if (cb_count.cnt !=6 ) { -printf ("Missing callbacks\n" ); -/* remove any callbacks that might remain */ -rte_eal_alarm_cancel (test_multi_cb, (void *)-1 ); -return -1 ; - } - - cb_count.cnt =0 ; -printf ("Expect only callbacks with args 1 and 3...\n" ); -/* Add 3 flags, then delete one */ -rte_eal_alarm_set (30 *US_PER_MS , test_multi_cb, (void *)3 ); -rte_eal_alarm_set (20 *US_PER_MS , test_multi_cb, (void *)2 ); -rte_eal_alarm_set (10 *US_PER_MS , test_multi_cb, (void *)1 ); - rm_count =rte_eal_alarm_cancel (test_multi_cb, (void *)2 ); - -rte_delay_ms (35 ); -if (cb_count.cnt !=2 || rm_count !=1 ) { -printf ("Error: invalid flags count or alarm removal failure" -" - flags value = %d, expected = %d\n" , - (int )cb_count.cnt,2 ); -/* remove any callbacks that might remain */ -rte_eal_alarm_cancel (test_multi_cb, (void *)-1 ); -return -1 ; - } - -printf ("Testing adding and then removing multiple alarms\n" ); -/* finally test that no callbacks are called if we delete them all*/ -rte_eal_alarm_set (10 *US_PER_MS , test_multi_cb, (void *)1 ); -rte_eal_alarm_set (10 *US_PER_MS , test_multi_cb, (void *)2 ); -rte_eal_alarm_set (10 *US_PER_MS , test_multi_cb, (void *)3 ); - rm_count =rte_eal_alarm_cancel (test_alarm_callback, (void *)-1 ); -if (rm_count !=0 ) { -printf ("Error removing non-existant alarm succeeded\n" ); -rte_eal_alarm_cancel (test_multi_cb, (void *)-1 ); -return -1 ; - } - rm_count =rte_eal_alarm_cancel (test_multi_cb, (void *)-1 ); -if (rm_count !=3 ) { -printf ("Error removing all pending alarm callbacks\n" ); -return -1 ; - } - -/* Test that we cannot cancel an alarm from within the callback itself -* Also test that we can cancel head-of-line callbacks ok.*/ - flag =0 ; - recursive_error =0 ; -rte_eal_alarm_set (10 *US_PER_MS , test_remove_in_callback, (void *)1 ); -rte_eal_alarm_set (20 *US_PER_MS , test_remove_in_callback, (void *)2 ); - rm_count =rte_eal_alarm_cancel (test_remove_in_callback, (void *)1 ); -if (rm_count !=1 ) { -printf ("Error cancelling head-of-list callback\n" ); -return -1 ; - } -rte_delay_ms (15 ); -if (flag !=0 ) { -printf ("Error, cancelling head-of-list leads to premature callback\n" ); -return -1 ; - } - -while (flag !=2 && count++ <RTE_TEST_MAX_REPEAT ) -rte_delay_ms (10 ); - -if (flag !=2 ) { -printf ("Error - expected callback not called\n" ); -rte_eal_alarm_cancel (test_remove_in_callback, (void *)-1 ); -return -1 ; - } -if (recursive_error ==1 ) -return -1 ; - -/* Check if it can cancel all for the same callback */ -printf ("Testing canceling all for the same callback\n" ); - flag_2 =0 ; -rte_eal_alarm_set (10 *US_PER_MS , test_remove_in_callback, (void *)1 ); -rte_eal_alarm_set (20 *US_PER_MS , test_remove_in_callback_2, (void *)2 ); -rte_eal_alarm_set (30 *US_PER_MS , test_remove_in_callback_2, (void *)3 ); -rte_eal_alarm_set (40 *US_PER_MS , test_remove_in_callback, (void *)4 ); - rm_count =rte_eal_alarm_cancel (test_remove_in_callback_2, (void *)-1 ); -if (rm_count !=2 ) { -printf ("Error, cannot cancel all for the same callback\n" ); -return -1 ; - } - rm_count =rte_eal_alarm_cancel (test_remove_in_callback, (void *)-1 ); -if (rm_count !=2 ) { -printf ("Error, cannot cancel all for the same callback\n" ); -return -1 ; - } - -return 0 ; -} -static int test_alarm (void ) { -int count =0 ;#ifdef RTE_EXEC_ENV_FREEBSD printf ("The alarm API is not supported on FreeBSD\n" );return 0 ;#endif -/* check if the callback will be called */ -printf ("check if the callback will be called\n" ); - flag =0 ; -if (rte_eal_alarm_set (RTE_TEST_ALARM_TIMEOUT *US_PER_MS , - test_alarm_callback,NULL ) <0 ) { -printf ("fail to set alarm callback\n" ); -return -1 ; - } -while (flag ==0 && count++ <RTE_TEST_MAX_REPEAT ) -rte_delay_ms (RTE_TEST_CHECK_PERIOD ); - -if (flag ==0 ){ -printf ("Callback not called\n" ); -return -1 ; - }/* check if it will fail to set alarm with wrong us value */ printf ("check if it will fail to set alarm with wrong ms values\n" );
@@ -225,9 +63,6 @@ test_alarm(void)return -1 ; } -if (test_multi_alarms () !=0 ) -return -1 ; -return 0 ; }
app/test/test_cycles.c +0 -78
84 lines not on this page · raw message
Signature · 1 line
2.31.1
Aaron Conole
NAKRe: [dpdk-dev] [PATCH v2] test: remove strict timing requirements from alarm and cycles tests
Quoted 10 lines · Aaron Conole
The tests '
alarm_autotest' and 'cycles_autotest' rely on the underlying system having very accurate and precise timing. On systems where the timing isn't as rigid, or the load is particularly high, these tests are unreliable since the wake latency from the scheduler can be high enough to miss the timing window.Remove the timing related tests from the test suites. These tests now ensure the add/remove callback infrastructure unit tests, but drop the waits and reliance on system timing and load.
Quoted 5 lines
This avoids FAIL on various testing infrastructures.
Signed-off-by Aaron Conole <[email protected]>---
v2: Drop the timing requirements, but keep the API calls
NAK - I have a v3 for this with some stuff that was missed.
PATCH v3 1/1 test: remove strict timing requirements from alarm and cycles tests
In reply to Aaron ConoleThe tests 'alarm_autotest' and 'cycles_autotest' rely on the underlying system having very accurate and precise timing. On systems where the timing isn't as rigid, or the load is particularly high, these tests are unreliable since the wake latency from the scheduler can be high enough to miss the timing window.
Remove the timing related tests from the test suites. These tests now ensure the add/remove callback infrastructure unit tests, but drop the waits and reliance on system timing and load.
This avoids FAIL on various testing infrastructures.
--- app/test/autotest_data.py | 12 --- app/test/meson.build | 2 - app/test/test_alarm.c | 176 +------------------------------------- app/test/test_cycles.c | 81 ------------------ 4 files changed, 1 insertion(+), 270 deletions(-)
app/test/autotest_data.py +0 -12
@@ -9,12 +9,6 @@ # groups of tests that can be run in parallel # the grouping has been found largely empirically parallel_test_list = [ - { - "Name": "Cycles autotest", - "Command": "cycles_autotest", - "Func": default_autotest, - "Report": None, - }, { "Name": "Timer autotest", "Command": "timer_autotest",
@@ -183,12 +177,6 @@ "Func": default_autotest, "Report": None, }, - { - "Name": "Alarm autotest", - "Command": "alarm_autotest", - "Func": default_autotest, - "Report": None, - }, { "Name": "Malloc autotest", "Command": "malloc_autotest",
app/test/meson.build +0 -2
@@ -185,14 +185,12 @@ test_deps = [ # to indicate whether it can run in no-huge mode. fast_tests = [ ['acl_autotest', true], - ['alarm_autotest', false], ['atomic_autotest', false], ['bitops_autotest', true], ['byteorder_autotest', true], ['cmdline_autotest', true], ['common_autotest', true], ['cpuflags_autotest', true], - ['cycles_autotest', true], ['debug_autotest', true], ['eal_flags_c_opt_autotest', false], ['eal_flags_main_opt_autotest', false],
app/test/test_alarm.c +1 -175
211 lines not on this page · raw message
app/test/test_cycles.c +0 -81
87 lines not on this page · raw message
Signature · 1 line
2.31.1
PATCH v4 1/1 test: remove strict timing requirements from alarm and cycles tests
The tests 'alarm_autotest' and 'cycles_autotest' rely on the underlying system having very accurate and precise timing. On systems where the timing isn't as rigid, or the load is particularly high, these tests are unreliable since the wake latency from the scheduler can be high enough to miss the timing window.
Remove the timing related tests from the test suites. These tests now ensure the add/remove callback infrastructure unit tests, but drop the waits and reliance on system timing and load.
This avoids FAIL on various testing infrastructures.
---
v4: I relied on the github actions test mechanism to do the test, but
it appears it was successful even though a run manually or in interactive mode would have revealed a failure. Additionally, the travis run for my local repo also uncovered the issue.
app/test/autotest_data.py | 18 ---- app/test/meson.build | 3 - app/test/test_alarm.c | 176 +------------------------------------- app/test/test_cycles.c | 81 ------------------ 4 files changed, 1 insertion(+), 277 deletions(-)
app/test/autotest_data.py +0 -18
@@ -9,12 +9,6 @@ # groups of tests that can be run in parallel # the grouping has been found largely empirically parallel_test_list = [ - { - "Name": "Cycles autotest", - "Command": "cycles_autotest", - "Func": default_autotest, - "Report": None, - }, { "Name": "Timer autotest", "Command": "timer_autotest",
@@ -183,12 +177,6 @@ "Func": default_autotest, "Report": None, }, - { - "Name": "Alarm autotest", - "Command": "alarm_autotest", - "Func": default_autotest, - "Report": None, - }, { "Name": "Malloc autotest", "Command": "malloc_autotest",
@@ -345,12 +333,6 @@ "Func": default_autotest, "Report": None, }, - { - "Name": "Sleep delay", - "Command": "delay_us_sleep_autotest", - "Func": default_autotest, - "Report": None, - }, { "Name": "Rawdev autotest", "Command": "rawdev_autotest",
app/test/meson.build +0 -3
@@ -185,14 +185,12 @@ test_deps = [ # to indicate whether it can run in no-huge mode. fast_tests = [ ['acl_autotest', true], - ['alarm_autotest', false], ['atomic_autotest', false], ['bitops_autotest', true], ['byteorder_autotest', true], ['cmdline_autotest', true], ['common_autotest', true], ['cpuflags_autotest', true], - ['cycles_autotest', true], ['debug_autotest', true], ['eal_flags_c_opt_autotest', false], ['eal_flags_main_opt_autotest', false],
@@ -255,7 +253,6 @@ fast_tests = [ ['user_delay_us', true], ['version_autotest', true], ['crc_autotest', true], - ['delay_us_sleep_autotest', true], ['distributor_autotest', false], ['eventdev_common_autotest', true], ['fbarray_autotest', true],
app/test/test_alarm.c +1 -175
211 lines not on this page · raw message
app/test/test_cycles.c +0 -81
87 lines not on this page · raw message
Signature · 1 line
2.31.1
David Marchand
AppliedRe: [dpdk-dev] [PATCH v4] test: remove strict timing requirements from alarm and cycles tests
Quoted 7 lines · Aaron Conole
The tests '
alarm_autotest' and 'cycles_autotest' rely on the underlying system having very accurate and precise timing. On systems where the timing isn't as rigid, or the load is particularly high, these tests are unreliable since the wake latency from the scheduler can be high enough to miss the timing window.
Quoted 7 lines
Remove the timing related tests from the test suites. These tests now ensure the add/remove callback infrastructure unit tests, but drop the waits and reliance on system timing and load.
This avoids FAIL on various testing infrastructures.
Signed-off-by Aaron Conole <[email protected]>
Applied, thanks Aaron.
Signature · 1 line
David Marchand