Fix yield test and remove few compile warnings#14
Conversation
|
|
||
| for (i = 1; i <= ITERATIONS; i++) { | ||
| gather_set1_stats(MAIN_PRIORITY, i); | ||
| bench_collect_resources(); |
There was a problem hiding this comment.
Is this right? The ancillary thread should never run, so it never adds itself to the "remove list".
There was a problem hiding this comment.
As @edersondisouza pointed out, the ancillary thread should never run. If it is actually running then the test is not working as intended and further investigation would be needed. The presence of bench_collect_resources() would then be hiding a problem.
| @@ -57,6 +57,7 @@ __weak void bench_exit_timer_isr(void) | |||
| */ | |||
There was a problem hiding this comment.
For the commit message: you give a more detailed description of the issue on the PR message - could you please add that info to the commit message? It's better to have more information in it.
|
|
||
| for (i = 1; i < ITERATIONS; i++) { | ||
| gather_set2_stats(MAIN_PRIORITY, i); | ||
| bench_collect_resources(); |
There was a problem hiding this comment.
Even here, IIUIC, the ancillary thread (bench_set2_helper) doesn't ever get to the bench_thread_exit(), it's aborted before - am I missing something?
There was a problem hiding this comment.
My debugging shows that bench_thread_exit is called 1000times (or ITERATIONS times). Which effectively leads to buffer overflow. (I used different FreeRTOS_config.h than the one provided)
I have followed how other tests were organized - gather_xx_stats, bench_collect_resources. When this same template was applied on the failing test, it started to work. I have not analyzed how gather_set[12]_stats works for the yield test.
The test framework should not assume what bench does on the inside.
There was a problem hiding this comment.
You indicated that a different configuration file was used. Can you identify the key differences?
There was a problem hiding this comment.
The issue occurs when configUSE_TIME_SLICING is defined 1 in FreeRTOSConfig.h . The bench_set2_helper() will run into bench_thread_exit() after the time slice of bench_thread_yield() is used up.
There was a problem hiding this comment.
Nice catch, I didn't notice this option, as it was not in my config file and defaults to being enabled by default. I didnt have enough time to get back to testing this.
4 participants
This PR fixes yield test with FreeRTOS.
This PR also does some cleaning: