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
> > ---
> > 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
> >
> > #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();
> > +
> > + 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
>
> -Michał
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
