Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Openposix pitest* useless #584

Closed
MofX opened this issue Sep 23, 2019 · 4 comments
Closed

Openposix pitest* useless #584

MofX opened this issue Sep 23, 2019 · 4 comments

Comments

@MofX
Copy link
Contributor

MofX commented Sep 23, 2019

All of the pitests in the openposix functional/threads testsuite are completely useless the way they are implemented at the moment. Here are my reasons:

  1. They are broken for systems with more than 7 cpus (pitest*: Allow more than 7 cpus #561)
  2. The test descriptions do not describe what the tests should test, but describe how they test
  3. They are ridiculously slow (over a minute)
    4.They do not check if the test really passed, there are just some checks for failed calls in there, Not even everything is checked (most importantly sched_setaffinity is not checked and will fail, if not run as root)
  4. The data has to be plot manually and inspected. This doesn't work on a moderately fast system, because the plotted counters wrap around multiple times (see below).
  5. The script to plot the data is not working on all platform, where sh != bash, because it uses bashisms and the shebang is just /bin/sh
  6. The README file talks about a run.sh script, that doesn't exist and the link to the image at the end is dead

About the useless plot on a moderate system:
Here you can see the plot of pitest-1 on my system. The readme says it should look like

If the pi function works, TF's progress should be a straight line
constantly going up; TP's progress should start at about 10 seconds
and should be parallel to TF's until ten seconds later (20 seconds)
when TL is created; at this time TP's slope should go down a wee bit
and TL's slope should not be zero. After 10 seconds (30 seconds), TB
is created and TL boosted. At this moment, TP's slope should go down
to zero and TL's should be parallel to TF's. After TB timeouts on
waiting the mutex (50 seconds), TL and TP's slope will change back to
the original trend during 20~30 seconds.

image
Because of wrapping of the counters I can't see anything described there.

Even after cleaning all the clutter (TF* counters) and changing to line plotting, you cannot see the line for TB:
image

I propose to remove the whole testsuite from the ltp tests, maybe documenting the actual subject under test, because the tests are quite useful, but not the way they are now.

Example descriptions for test 1 and 2 combined:

  1. Test priority boosting for a thread, that holds a mutex, while another thread wants to look the same mutex
  2. Test priority boosting for two individual sets of threads like in A, to prevent interaction between the mutexes

The actual tests could be somewhat like the original pitests: Counters that are checked for plausible values at different stages during the test.

BTW: pitest 1-4 are broken at the moment for systems with just one core, or if they are bound to a single core (taskset 0x01 ...). See: https://github.com/MofX/pthread_mutex_timedlock_prio_bug

@metan-ucw
Copy link
Member

Sounds about right, I guess the next step should be sending a commit with this description, that would remove these tests to the LTP mailing list, which is where most of the communication happens. If nobody would complain there we can just merge the removal.

@MofX
Copy link
Contributor Author

MofX commented Nov 15, 2019

Alright, I will send it next week.

@jstancek
Copy link
Contributor

No objection to dropping it. https://git.kernel.org/pub/scm/utils/rt-tests/rt-tests.git/tree/src/pi_tests could be a better alternative for people interested in testing PI mutexes.

@MofX
Copy link
Contributor Author

MofX commented Nov 21, 2019

I sent a patch to the mailing list on 2019-11-18

pevik pushed a commit to pevik/ltp that referenced this issue Nov 21, 2019
There are several reasons, why these tests should be removed.

Most important: They do not really test anything. The result of these
tests has to be inspected manually do check, if the behavior is correct,
but the manual inspection using the given tools is not possible, because
these tests were written for systems much slower than todays systems.

The problems are described in more detail in
linux-test-project#584

Reviewed-by: Petr Vorel <pvorel@suse.cz>
Acked-by: Cyril Hrubis <chrubis@suse.cz>
Acked-by: Jan Stancek <jstancek@redhat.com>
Signed-off-by: Joerg Vehlow <joerg.vehlow@aox-tech.de>
Signed-off-by: Petr Vorel <pvorel@suse.cz>
@pevik pevik closed this as completed in ee829ec Nov 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants