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

[Janusz Krzysztofik]

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 at 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 at intel.com>
> 
> -Michał