Re: [igt-dev] [Intel-gfx] [RFC PATCH i-g-t v2 8/8] tests/core_hotunplug: Add 'GEM batch' variant

[Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>]

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

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev