commit 9250c0ee2626 ("greybus: Loopback_test: use poll instead of inotify") changes the flow of determining when to break out of a loop polling for loopback test completion.
The clause is_complete() which determines if all tests are complete - as used is subject to a race condition where one of the tests has completed but at least one other test has not. On real hardware this typically doesn't present itself however in gbsim - which is a lot slower due in-part to a panopoly of printouts - we see that running a loopback test to more than one Interface in gbsim will fail in all instances printing out "Iteration count did not finish".
We don't even need to have the poll() loop timeout tests in since the kernel threads themselves should eventually complete - and if not it's a kernel bug. A user of the tool can still terminate tests by doing a ctrl-c.
This patch fixes the race by ensuring the poll() loop waits for all active Interfaces to complete - as opposed to the first active Interface before breaking the loop.
Signed-off-by: Bryan O'Donoghue pure.logic@nexus-software.ie --- drivers/staging/greybus/tools/loopback_test.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/staging/greybus/tools/loopback_test.c b/drivers/staging/greybus/tools/loopback_test.c index f7f4cd6..f053d4fe 100644 --- a/drivers/staging/greybus/tools/loopback_test.c +++ b/drivers/staging/greybus/tools/loopback_test.c @@ -747,7 +747,7 @@ static int wait_for_complete(struct loopback_test *t) if (t->poll_timeout.tv_sec != 0) ts = &t->poll_timeout;
- while (1) { + while (!is_complete(t)) {
ret = ppoll(t->fds, t->poll_count, ts, &mask_old); if (ret <= 0) { @@ -763,14 +763,6 @@ static int wait_for_complete(struct loopback_test *t) number_of_events++; } } - - if (number_of_events == t->poll_count) - break; - } - - if (!is_complete(t)) { - fprintf(stderr, "Iteration count did not finish!\n"); - return -1; }
return 0;