| Message ID | 20200622164415.30352-6-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:12) > Verify if an additional address space associated with an open device > file descriptor is cleaned up correctly on device hotunplug. > > v2: rebase on upstream, update includes order > > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> > --- > tests/core_hotunplug.c | 31 +++++++++++++++++++++++++++++++ > 1 file changed, 31 insertions(+) > > diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c > index 0892e1927..18a963564 100644 > --- a/tests/core_hotunplug.c > +++ b/tests/core_hotunplug.c > @@ -30,6 +30,7 @@ > #include <unistd.h> > > #include "i915/gem.h" > +#include "i915/gem_vm.h" > #include "igt.h" > #include "igt_device_scan.h" > #include "igt_kmod.h" > @@ -332,6 +333,29 @@ static void hotreplug_lateclose(void) > healthcheck(); > } > > +static void vm_hotunplug_lateclose(void) > +{ > + struct hotunplug priv; > + > + prepare_for_rescan(&priv); > + > + gem_require_vm(priv.fd.drm); > + > + local_debug("creating additional GEM user address space"); > + igt_ignore_warn(gem_vm_create(priv.fd.drm)); Why the "ignore_warn"? This deserves a comment. And perhaps a word of information about why we picked this partucular operation in the commit message (vm_create). This is a regression test, right? LGTM otherwise (but again - see previous patches): Reviewed-by: Michał Winiarski <michal.winiarski@intel.com> -Michał
Hi Michał, thanks for review. On Thu, 2020-06-25 at 21:42 +0200, Michał Winiarski wrote: > Quoting Janusz Krzysztofik (2020-06-22 18:44:12) > > Verify if an additional address space associated with an open device > > file descriptor is cleaned up correctly on device hotunplug. > > > > v2: rebase on upstream, update includes order > > > > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> > > --- > > tests/core_hotunplug.c | 31 +++++++++++++++++++++++++++++++ > > 1 file changed, 31 insertions(+) > > > > diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c > > index 0892e1927..18a963564 100644 > > --- a/tests/core_hotunplug.c > > +++ b/tests/core_hotunplug.c > > @@ -30,6 +30,7 @@ > > #include <unistd.h> > > > > #include "i915/gem.h" > > +#include "i915/gem_vm.h" > > #include "igt.h" > > #include "igt_device_scan.h" > > #include "igt_kmod.h" > > @@ -332,6 +333,29 @@ static void hotreplug_lateclose(void) > > healthcheck(); > > } > > > > +static void vm_hotunplug_lateclose(void) > > +{ > > + struct hotunplug priv; > > + > > + prepare_for_rescan(&priv); > > + > > + gem_require_vm(priv.fd.drm); > > + > > + local_debug("creating additional GEM user address space"); > > + igt_ignore_warn(gem_vm_create(priv.fd.drm)); > > Why the "ignore_warn"? This deserves a comment. > And perhaps a word of information about why we picked > this partucular operation in the commit message (vm_create). > This is a regression test, right? Hmm, I didn't intend it to be a regression test. The purpose was generally the same as of other hotunplug-lateclose subtests - exercise the driver behaviour on hotunplug and lateclose. This subtest was intended to perform still the same exercise, only under different conditions, or different use case of the driver. In particular, hotunplug is performed here with an additional address space associated with an open file descriptor. We are not interested in exercising that address space (that's why igt_ignore_warn), only in checking if it is cleaned up on time so hotunplug-lateclose operations are still safe from late dma_unmap issues. Let me try to reword the commit description so it better reflects this idea. Thanks, Janusz > > LGTM otherwise (but again - see previous patches): > > Reviewed-by: Michał Winiarski <michal.winiarski@intel.com> > > -Michał
diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c index 0892e1927..18a963564 100644 --- a/tests/core_hotunplug.c +++ b/tests/core_hotunplug.c @@ -30,6 +30,7 @@ #include <unistd.h> #include "i915/gem.h" +#include "i915/gem_vm.h" #include "igt.h" #include "igt_device_scan.h" #include "igt_kmod.h" @@ -332,6 +333,29 @@ static void hotreplug_lateclose(void) healthcheck(); } +static void vm_hotunplug_lateclose(void) +{ + struct hotunplug priv; + + prepare_for_rescan(&priv); + + gem_require_vm(priv.fd.drm); + + local_debug("creating additional GEM user address space"); + igt_ignore_warn(gem_vm_create(priv.fd.drm)); + + 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 @@ -404,4 +428,11 @@ igt_main igt_fixture igt_abort_on_f(failure, "%s\n", failure); + + igt_describe("Check if a still open device with extra GEM address space can be cleanly unplugged, then released and recovered"); + igt_subtest("vm-hotunplug-lateclose") + vm_hotunplug_lateclose(); + + igt_fixture + igt_abort_on_f(failure, "%s\n", failure); }
Verify if an additional address space associated with an open device file descriptor is cleaned up correctly on device hotunplug. v2: rebase on upstream, update includes order Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com> --- tests/core_hotunplug.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)