[RFC,i-g-t,v2,6/8] tests/core_hotunplug: Add 'GEM object' variant
diff mbox series

Message ID 20200622164415.30352-7-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
GEM objects belonging to user file descriptors still open on device
hotunplug may exhibit still more driver issues.  Add a subtest that
implements this scenario.

v2: rebase on upstream

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

Comments

Michał Winiarski June 25, 2020, 7:51 p.m. UTC | #1
Quoting Janusz Krzysztofik (2020-06-22 18:44:13)
> GEM objects belonging to user file descriptors still open on device
> hotunplug may exhibit still more driver issues.  Add a subtest that
> implements this scenario.
> 
> v2: rebase on upstream
> 
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> ---
>  tests/core_hotunplug.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> index 18a963564..c30d98a69 100644
> --- a/tests/core_hotunplug.c
> +++ b/tests/core_hotunplug.c
> @@ -356,6 +356,29 @@ static void vm_hotunplug_lateclose(void)
>         healthcheck();
>  }
>  
> +static void gem_hotunplug_lateclose(void)
> +{
> +       struct hotunplug priv;
> +
> +       prepare_for_rescan(&priv);
> +
> +       igt_require_gem(priv.fd.drm);
> +
> +       local_debug("creating a GEM user object");
> +       igt_ignore_warn(gem_create(priv.fd.drm, 4096));

Same as previous one.
(note - we could just check for proper error when we attempt to close this
handle after unplug, and the same thing applies to the previous one with the vm)

LGTM otherwise.

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

-Michał
Janusz Krzysztofik June 26, 2020, 8:26 a.m. UTC | #2
Hi Michał,

Thanks for review.

On Thu, 2020-06-25 at 21:51 +0200, Michał Winiarski wrote:
> Quoting Janusz Krzysztofik (2020-06-22 18:44:13)
> > GEM objects belonging to user file descriptors still open on device
> > hotunplug may exhibit still more driver issues.  Add a subtest that
> > implements this scenario.
> > 
> > v2: rebase on upstream
> > 
> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> > ---
> >  tests/core_hotunplug.c | 30 ++++++++++++++++++++++++++++++
> >  1 file changed, 30 insertions(+)
> > 
> > diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> > index 18a963564..c30d98a69 100644
> > --- a/tests/core_hotunplug.c
> > +++ b/tests/core_hotunplug.c
> > @@ -356,6 +356,29 @@ static void vm_hotunplug_lateclose(void)
> >         healthcheck();
> >  }
> >  
> > +static void gem_hotunplug_lateclose(void)
> > +{
> > +       struct hotunplug priv;
> > +
> > +       prepare_for_rescan(&priv);
> > +
> > +       igt_require_gem(priv.fd.drm);
> > +
> > +       local_debug("creating a GEM user object");
> > +       igt_ignore_warn(gem_create(priv.fd.drm, 4096));
> 
> Same as previous one.
> (note - we could just check for proper error when we attempt to close this
> handle after unplug, and the same thing applies to the previous one with the vm)

Oh, now I see what you meant in case of the address space variant.

I was thinking about that.  We may need another subtests, or a group of
subtests, for exercising the driver's safety from post-hotunplug
attempts to use the removed device, not only to close it.  I decided to
provide those variants later and call them 'hotunplug-lateuse*'.

However, now I see that we may perfectly exercise the driver's
resistance to late use of additional user resources while having those
resources already created.  Then, let me extend applicable members of
this patch series with those checks.

Thanks,
Janusz

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

Patch
diff mbox series

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 18a963564..c30d98a69 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -356,6 +356,29 @@  static void vm_hotunplug_lateclose(void)
 	healthcheck();
 }
 
+static void gem_hotunplug_lateclose(void)
+{
+	struct hotunplug priv;
+
+	prepare_for_rescan(&priv);
+
+	igt_require_gem(priv.fd.drm);
+
+	local_debug("creating a GEM user object");
+	igt_ignore_warn(gem_create(priv.fd.drm, 4096));
+
+	local_debug("hot unplugging the device");
+	device_unplug(priv.fd.sysfs_dev);
+
+	local_debug("late closing the removed device instance");
+	close(priv.fd.drm);
+
+	local_debug("recovering the device");
+	bus_rescan(priv.fd.sysfs_bus);
+
+	healthcheck();
+}
+
 /* Main */
 
 igt_main
@@ -435,4 +458,11 @@  igt_main
 
 	igt_fixture
 		igt_abort_on_f(failure, "%s\n", failure);
+
+	igt_describe("Check if a device with a still open GEM object can be cleanly unplugged, then released and recovered");
+	igt_subtest("gem-hotunplug-lateclose")
+		gem_hotunplug_lateclose();
+
+	igt_fixture
+		igt_abort_on_f(failure, "%s\n", failure);
 }