[RFC,i-g-t,v2,7/8] tests/core_hotunplug: Add 'PRIME handle' variant
diff mbox series

Message ID 20200622164415.30352-8-janusz.krzysztofik@linux.intel.com
State New
Headers show
Series
  • tests/core_hotunplug: New subtests and enhancements
Related show

Commit Message

Janusz Krzysztofik June 22, 2020, 4:44 p.m. UTC
Even if all device file descriptors are closed on device hotunplug,
PRIME exported objects may still exists, referenced by still open
dma-buf file handles.  Add a subtest that keeps such handle open on
device hotunplug.

v2: rebase on upstream

Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
---
 tests/core_hotunplug.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

Comments

Michał Winiarski June 25, 2020, 7:57 p.m. UTC | #1
Quoting Janusz Krzysztofik (2020-06-22 18:44:14)
> Even if all device file descriptors are closed on device hotunplug,
> PRIME exported objects may still exists, referenced by still open
> dma-buf file handles.  Add a subtest that keeps such handle open on
> device hotunplug.
> 
> v2: rebase on upstream

Would be interesting to see what happens when someone actually imports an
object from unplugged device (or the device is unplugged after it was imported).
But perhaps that's something for the future.

Also - the naming should probably be kept distinct from the other "lateclose"
tests, since here we're closing the device FD before the unplug.
Maybe just "prime-hotunplug"? But that's up to you - either way:

Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>

-Michał

> 
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> ---
>  tests/core_hotunplug.c | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> index c30d98a69..7cb699cc2 100644
> --- a/tests/core_hotunplug.c
> +++ b/tests/core_hotunplug.c
> @@ -379,6 +379,35 @@ static void gem_hotunplug_lateclose(void)
>         healthcheck();
>  }
>  
> +static void prime_hotunplug_lateclose(void)
> +{
> +       struct hotunplug priv;
> +       uint32_t handle;
> +       int dmabuf;
> +
> +       prepare_for_rescan(&priv);
> +
> +       igt_require_gem(priv.fd.drm);
> +
> +       local_debug("creating and PRIME-exporting a GEM object");
> +       handle = gem_create(priv.fd.drm, 4096);
> +       dmabuf = prime_handle_to_fd(priv.fd.drm, handle);
> +
> +       local_debug("closing the device");
> +       close(priv.fd.drm);
> +
> +       local_debug("hot unplugging the device");
> +       device_unplug(priv.fd.sysfs_dev);
> +
> +       local_debug("late closing the PRIME file handle");
> +       close(dmabuf);
> +
> +       local_debug("recovering the device");
> +       bus_rescan(priv.fd.sysfs_bus);
> +
> +       healthcheck();
> +}
> +
>  /* Main */
>  
>  igt_main
> @@ -465,4 +494,11 @@ igt_main
>  
>         igt_fixture
>                 igt_abort_on_f(failure, "%s\n", failure);
> +
> +       igt_describe("Check if a device with a still open PRIME-exported object can be cleanly unplugged, then released and recovered");
> +       igt_subtest("prime-hotunplug-lateclose")
> +               prime_hotunplug_lateclose();
> +
> +       igt_fixture
> +               igt_abort_on_f(failure, "%s\n", failure);
>  }
> -- 
> 2.21.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
Janusz Krzysztofik June 26, 2020, 10:56 a.m. UTC | #2
Hi Michał,

On Thu, 2020-06-25 at 21:57 +0200, Michał Winiarski wrote:
> Quoting Janusz Krzysztofik (2020-06-22 18:44:14)
> > Even if all device file descriptors are closed on device hotunplug,
> > PRIME exported objects may still exists, referenced by still open
> > dma-buf file handles.  Add a subtest that keeps such handle open on
> > device hotunplug.
> > 
> > v2: rebase on upstream
> 
> Would be interesting to see what happens when someone actually imports an
> object from unplugged device (or the device is unplugged after it was imported).
> But perhaps that's something for the future.

Yes, let's keep it relatively simple for now.  There seems to be quite
a few possible scenarios to cover.   However, I'm going to add a very
basic use-after-hotunplug check, similar to what we have (hopefully)
agreed for context and address space variants.

> 
> Also - the naming should probably be kept distinct from the other "lateclose"
> tests, since here we're closing the device FD before the unplug.
> Maybe just "prime-hotunplug"? 

Since we are still interested in exercising the driver behaviour on
late closing the prime handle (now this case also explodes inside
intel-iommu), let's keep that namig even if we close the device and
only keep the prime file open.

Thanks,
Janusz


> But that's up to you - either way:
> 
> Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
> 
> -Michał
> 
> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> > ---
> >  tests/core_hotunplug.c | 36 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 36 insertions(+)
> > 
> > diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> > index c30d98a69..7cb699cc2 100644
> > --- a/tests/core_hotunplug.c
> > +++ b/tests/core_hotunplug.c
> > @@ -379,6 +379,35 @@ static void gem_hotunplug_lateclose(void)
> >         healthcheck();
> >  }
> >  
> > +static void prime_hotunplug_lateclose(void)
> > +{
> > +       struct hotunplug priv;
> > +       uint32_t handle;
> > +       int dmabuf;
> > +
> > +       prepare_for_rescan(&priv);
> > +
> > +       igt_require_gem(priv.fd.drm);
> > +
> > +       local_debug("creating and PRIME-exporting a GEM object");
> > +       handle = gem_create(priv.fd.drm, 4096);
> > +       dmabuf = prime_handle_to_fd(priv.fd.drm, handle);
> > +
> > +       local_debug("closing the device");
> > +       close(priv.fd.drm);
> > +
> > +       local_debug("hot unplugging the device");
> > +       device_unplug(priv.fd.sysfs_dev);
> > +
> > +       local_debug("late closing the PRIME file handle");
> > +       close(dmabuf);
> > +
> > +       local_debug("recovering the device");
> > +       bus_rescan(priv.fd.sysfs_bus);
> > +
> > +       healthcheck();
> > +}
> > +
> >  /* Main */
> >  
> >  igt_main
> > @@ -465,4 +494,11 @@ igt_main
> >  
> >         igt_fixture
> >                 igt_abort_on_f(failure, "%s\n", failure);
> > +
> > +       igt_describe("Check if a device with a still open PRIME-exported object can be cleanly unplugged, then released and recovered");
> > +       igt_subtest("prime-hotunplug-lateclose")
> > +               prime_hotunplug_lateclose();
> > +
> > +       igt_fixture
> > +               igt_abort_on_f(failure, "%s\n", failure);
> >  }
> > -- 
> > 2.21.1
> > 
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev

Patch
diff mbox series

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index c30d98a69..7cb699cc2 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -379,6 +379,35 @@  static void gem_hotunplug_lateclose(void)
 	healthcheck();
 }
 
+static void prime_hotunplug_lateclose(void)
+{
+	struct hotunplug priv;
+	uint32_t handle;
+	int dmabuf;
+
+	prepare_for_rescan(&priv);
+
+	igt_require_gem(priv.fd.drm);
+
+	local_debug("creating and PRIME-exporting a GEM object");
+	handle = gem_create(priv.fd.drm, 4096);
+	dmabuf = prime_handle_to_fd(priv.fd.drm, handle);
+
+	local_debug("closing the device");
+	close(priv.fd.drm);
+
+	local_debug("hot unplugging the device");
+	device_unplug(priv.fd.sysfs_dev);
+
+	local_debug("late closing the PRIME file handle");
+	close(dmabuf);
+
+	local_debug("recovering the device");
+	bus_rescan(priv.fd.sysfs_bus);
+
+	healthcheck();
+}
+
 /* Main */
 
 igt_main
@@ -465,4 +494,11 @@  igt_main
 
 	igt_fixture
 		igt_abort_on_f(failure, "%s\n", failure);
+
+	igt_describe("Check if a device with a still open PRIME-exported object can be cleanly unplugged, then released and recovered");
+	igt_subtest("prime-hotunplug-lateclose")
+		prime_hotunplug_lateclose();
+
+	igt_fixture
+		igt_abort_on_f(failure, "%s\n", failure);
 }