| Message ID | 20200622164415.30352-7-janusz.krzysztofik@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tests/core_hotunplug: New subtests and enhancements | expand |
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ł
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ł
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); }
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(+)