[igt-dev] [PATCH] tests: Add a new test for driver/device hot remove

[Krzysztofik, Janusz]

Hi Petri,

On Friday, March 29, 2019 11:47:32 AM CET Petri Latvala wrote:
> On Thu, Mar 28, 2019 at 05:47:19PM +0100, Janusz Krzysztofik wrote:
> > Run a dummy load in background to put some workload on a device, then try
> > to either remove (unplug) the device from its bus, or unbind the device's
> > driver from it, depending on which subtest has been selected.
> > 
> > The driver hot unbind / device hot unplug operation is expected to succeed
> > in a reasonable time, however long timeouts are used to allow kernel
> > level timeouts pop up first if any.
> > 
> > Please note that if running both subtests consecutively, the second one
> > is always skipped as the device is no longer available as soon as the
> > first subtest is completed.  The most reliable way to run another subtest
> > is to reboot the system first, then select next subtest to be run.
> 
> This is a requirement that won't fly for CI use.

As far as I know, selftests are used in a similar way, requiring reboot after 
each subtest for reliable results.

> Is the
> rebinding/whatever of the device not possible to do? By the test?

See below.

> Does it also apply to running other test binaries after running the
> first subtest?

Yes, it does.

> As in, is it just a timing issue?

See below.

> > +	igt_subtest("unplug") {
> > +		igt_spin_t *spin;
> > +
> > +		/* Verify if a suitable DRM device/driver exists */
> > +		igt_skip_on(device < 0);
> > +
> > +		/* Run background workload */
> > +		spin = igt_spin_batch_new(device, .flags = 
IGT_SPIN_POLL_RUN);
> 
> igt_spin_batch_new requires DRIVER_INTEL, doesn't it?

Yes, it does.  The test will be just skipped on non-Intel drivers, I believe, 
unless they provide their own igt_spin_batch_new implementation.

> > +		igt_spin_busywait_until_running(spin);
> > +
> > +		/* Make the device disappear */
> > +		igt_set_timeout(60, "Device unplug timeout!");
> > +		device_unplug(device);
> > +		igt_reset_timeout();
> > +
> > +		close(device);
> > +		device = -1;
> > +	}
> > +
> > +	igt_subtest("unbind") {
> > +		igt_spin_t *spin;
> > +
> > +		/* Verify if a suitable DRM device/driver exists */
> > +		igt_skip_on(device < 0);
> 
> Ah, I see. The skip if running consecutively is because the previous
> subtest closed the fd.

There is no point in keeping it open as it is associated with a device which 
no longer exists.

> Another fixture before this subtest, get the device plugged back,
> reopen driver?

What should happen if that fails?  Or how to verify health of the re-plugged 
or re-bound device?

I think the problem is, a device which has just been hot removed only 
virtually and can't be reinitialized from scratch while being physically re-
plugged back may be in an unpredictable state and may not initialize correctly 
with the driver again.  If I tried to do that, I think I would extend the 
scope of this test with a check how rebinding the driver to a device in an 
unknown state works.  That's a candidate for a separate test, I believe.

I think it's better to explicitly require reboot for the device to re-appear 
than risk other (sub)tests are affected.

Thanks,
Janusz
--------------------------------------------------------------------

Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.