Re: [PATCH 2/4] selftests/vfio: Wait out transient -EBUSY on open/bind

[Alex Williamson]

On Wed, 9 Sep 2026 21:40:53 +0000
David Matlack <dmatlack@xxxxxxxxxx> wrote:

> On 2026-09-01 03:53 PM, Alex Williamson wrote:
> > If a test is killed, for example due to timeout, fput can be delayed,
> > allowing the subsequent test to be started while the failing test still
> > holds the device open count elevated.  This results in a cascade of
> > failures as each subsequent test fails on open, blocked by the single
> > user requirement at the group or device cdev file.
> > 
> > We can make the test framework more robust, and allow better
> > identification of specific failing scenarios, by waiting-out transient
> > -EBUSY failures on group open and cdev bind.
> > 
> > The 20s retry window is heuristically determined in testing on a system
> > where scheduling can be significantly delayed due to SMI handling of
> > platform errors generated from the mix-and-match test.
> > 
> > The SR-IOV uAPI and IOMMUFD setup tests retain their non-retry bind
> > paths as these are not expected to encounter process kills due to
> > underlying platform error handling variability.
> > 
> > Assisted-by: Qwen3.8-27B
> > Signed-off-by: Alex Williamson <alex.williamson@xxxxxxxxxx>  
> 
> I think we should fix this in kselftest_harness.h instead.
> 
> If the poll() times out the harness issues SIGKILL but then does not
> wait for the process to finish exiting (waitpid(..., WNOHANG). If we
> drop WNOHANG then the harness will wait for the killed process to exit
> before proceeding with the next test case.
> 
> diff --git a/tools/testing/selftests/kselftest_harness.h b/tools/testing/selftests/kselftest_harness.h
> index 1e459619fe86..150bd690ca7b 100644
> --- a/tools/testing/selftests/kselftest_harness.h
> +++ b/tools/testing/selftests/kselftest_harness.h
> @@ -1008,7 +1008,7 @@ static void __wait_for_test(struct __test_metadata *t)
>          /* signal process group */
>          kill(-(t->pid), SIGKILL);
>      }
> -    child = waitpid(t->pid, &status, WNOHANG);
> +    child = waitpid(t->pid, &status, 0);
>      if (child == -1 && errno != EINTR) {
>          t->exit_code = KSFT_FAIL;
>          fprintf(TH_LOG_STREAM,
> 

I had done something like this when debugging, but found it
insufficient.  More digging turns up why.  t->pid only runs the
TEST_F() wrapper, another child process is actually forked to run the
fixture test.  The kill(-(t->pid), SIGKILL) at the top of the diff
signals the whole process group, but dropping WNOHANG here only reaps
the wrapper process.  The grandchild process that actually holds the
vfio device files that require exclusive ownership is released
asynchronously.

Therefore we actually need to wait(2) for the wrapper process to exit
to reap the zombie, and then poll the process group with kill(, 0)
until it no longer exists, while not introducing an unbounded loop into
the harness.  Only then do we avoid the cascade failures.  And then we
don't need this retry on -EBUSY patch.

I'll send a patch for this outside of the vfio series since the cascade
failure is also resolved if we just extend the mix_and_match test to
avoid the timeout in the first place.  Thanks,

Alex