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

Message ID 20200622164415.30352-9-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
Verify if a device with a GEM batch job still running on a GPU can be
hot-unplugged cleanly and released, then recovered.

v2: rebase on upstream

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

Comments

Michał Winiarski June 25, 2020, 8:02 p.m. UTC | #1
Quoting Janusz Krzysztofik (2020-06-22 18:44:15)
> Verify if a device with a GEM batch job still running on a GPU can be
> hot-unplugged cleanly and released, then recovered.
> 
> v2: rebase on upstream
> 
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> ---
>  tests/core_hotunplug.c | 34 ++++++++++++++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
> 
> diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> index 7cb699cc2..672ff661d 100644
> --- a/tests/core_hotunplug.c
> +++ b/tests/core_hotunplug.c
> @@ -33,6 +33,7 @@
>  #include "i915/gem_vm.h"
>  #include "igt.h"
>  #include "igt_device_scan.h"
> +#include "igt_dummyload.h"
>  #include "igt_kmod.h"
>  #include "igt_sysfs.h"
>  
> @@ -408,6 +409,32 @@ static void prime_hotunplug_lateclose(void)
>         healthcheck();
>  }
>  
> +static void batch_hotunplug_lateclose(void)
> +{
> +       struct hotunplug priv;
> +       igt_spin_t *spin;
> +
> +       prepare_for_rescan(&priv);
> +
> +       igt_require_gem(priv.fd.drm);
> +
> +       local_debug("running dummy load");
> +       spin = __igt_spin_new(priv.fd.drm, .flags = IGT_SPIN_POLL_RUN |
> +                                                   IGT_SPIN_NO_PREEMPTION);

Do we need IGT_SPIN_NO_PREEMPTION here?
We're also leaking spin here... And I don't think we can just call igt_spin_free
after unplug, can we?

-Michał

> +       igt_spin_busywait_until_started(spin);
> +
> +       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
> @@ -501,4 +528,11 @@ igt_main
>  
>         igt_fixture
>                 igt_abort_on_f(failure, "%s\n", failure);
> +
> +       igt_describe("Check if a device with a still running batch can be cleanly unplugged, then released and recovered");
> +       igt_subtest("batch-hotunplug-lateclose")
> +               batch_hotunplug_lateclose();
> +
> +       igt_fixture
> +               igt_abort_on_f(failure, "%s\n", failure);
>  }
> -- 
> 2.21.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Janusz Krzysztofik June 26, 2020, 8:51 a.m. UTC | #2
Hi Michał,

On Thu, 2020-06-25 at 22:02 +0200, Michał Winiarski wrote:
> Quoting Janusz Krzysztofik (2020-06-22 18:44:15)
> > Verify if a device with a GEM batch job still running on a GPU can be
> > hot-unplugged cleanly and released, then recovered.
> > 
> > v2: rebase on upstream
> > 
> > Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> > ---
> >  tests/core_hotunplug.c | 34 ++++++++++++++++++++++++++++++++++
> >  1 file changed, 34 insertions(+)
> > 
> > diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
> > index 7cb699cc2..672ff661d 100644
> > --- a/tests/core_hotunplug.c
> > +++ b/tests/core_hotunplug.c
> > @@ -33,6 +33,7 @@
> >  #include "i915/gem_vm.h"
> >  #include "igt.h"
> >  #include "igt_device_scan.h"
> > +#include "igt_dummyload.h"
> >  #include "igt_kmod.h"
> >  #include "igt_sysfs.h"
> >  
> > @@ -408,6 +409,32 @@ static void prime_hotunplug_lateclose(void)
> >         healthcheck();
> >  }
> >  
> > +static void batch_hotunplug_lateclose(void)
> > +{
> > +       struct hotunplug priv;
> > +       igt_spin_t *spin;
> > +
> > +       prepare_for_rescan(&priv);
> > +
> > +       igt_require_gem(priv.fd.drm);
> > +
> > +       local_debug("running dummy load");
> > +       spin = __igt_spin_new(priv.fd.drm, .flags = IGT_SPIN_POLL_RUN |
> > +                                                   IGT_SPIN_NO_PREEMPTION);
> 
> Do we need IGT_SPIN_NO_PREEMPTION here?

Assuming my understanding of IGT_SPIN_NO_PREEMPTION was correct, my
intention was to exercise the driver's ability to cancel persistent GPU
tasks on hotunplug and clean up their associated resources on time in
order to avoid late dma_unmap issues.  Please advise if you still think
this this flag not needed.

> We're also leaking spin here... And I don't think we can just call igt_spin_free
> after unplug, can we?

If you mean memory occupied by the spin structure, I know leaking it
looks bad but I think that shouldn't be a problem in a user space
process that is going to exit soon.  But ayway, let me try what happens
on late igt_spin_free attempt.

Thanks,
Janusz

> 
> -Michał
> 
> > +       igt_spin_busywait_until_started(spin);
> > +
> > +       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
> > @@ -501,4 +528,11 @@ igt_main
> >  
> >         igt_fixture
> >                 igt_abort_on_f(failure, "%s\n", failure);
> > +
> > +       igt_describe("Check if a device with a still running batch can be cleanly unplugged, then released and recovered");
> > +       igt_subtest("batch-hotunplug-lateclose")
> > +               batch_hotunplug_lateclose();
> > +
> > +       igt_fixture
> > +               igt_abort_on_f(failure, "%s\n", failure);
> >  }
> > -- 
> > 2.21.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Patch
diff mbox series

diff --git a/tests/core_hotunplug.c b/tests/core_hotunplug.c
index 7cb699cc2..672ff661d 100644
--- a/tests/core_hotunplug.c
+++ b/tests/core_hotunplug.c
@@ -33,6 +33,7 @@ 
 #include "i915/gem_vm.h"
 #include "igt.h"
 #include "igt_device_scan.h"
+#include "igt_dummyload.h"
 #include "igt_kmod.h"
 #include "igt_sysfs.h"
 
@@ -408,6 +409,32 @@  static void prime_hotunplug_lateclose(void)
 	healthcheck();
 }
 
+static void batch_hotunplug_lateclose(void)
+{
+	struct hotunplug priv;
+	igt_spin_t *spin;
+
+	prepare_for_rescan(&priv);
+
+	igt_require_gem(priv.fd.drm);
+
+	local_debug("running dummy load");
+	spin = __igt_spin_new(priv.fd.drm, .flags = IGT_SPIN_POLL_RUN |
+						    IGT_SPIN_NO_PREEMPTION);
+	igt_spin_busywait_until_started(spin);
+
+	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
@@ -501,4 +528,11 @@  igt_main
 
 	igt_fixture
 		igt_abort_on_f(failure, "%s\n", failure);
+
+	igt_describe("Check if a device with a still running batch can be cleanly unplugged, then released and recovered");
+	igt_subtest("batch-hotunplug-lateclose")
+		batch_hotunplug_lateclose();
+
+	igt_fixture
+		igt_abort_on_f(failure, "%s\n", failure);
 }