Re: [Intel-gfx] [PATCH igt] lib/dummyload: Avoid assertions in lowlevel spin constructor

[Chris Wilson]

Quoting Antonio Argenziano (2018-02-22 21:24:31)
> 
> 
> On 22/02/18 00:50, Chris Wilson wrote:
> > __igt_spin_batch_new() may be used inside a background helper which is
> > competing against the GPU being reset. As such, we cannot even assert
> > that the spin->handle is busy immediately after submission as it may
> > have already been reset by another client writing to i915_wedged.
> > 
> > Signed-off-by: Chris Wilson 
> > ---
> >   lib/igt_dummyload.c | 17 ++---
> >   tests/pm_rps.c  |  4 +++-
> >   2 files changed, 17 insertions(+), 4 deletions(-)
> > 
> > diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
> > index 27eb402b..8ecd1dc8 100644
> > --- a/lib/igt_dummyload.c
> > +++ b/lib/igt_dummyload.c
> > @@ -25,6 +25,7 @@
> >   #include 
> >   #include 
> >   #include 
> > +#include 
> >   
> >   #include 
> >   
> > @@ -205,7 +206,6 @@ ___igt_spin_batch_new(int fd, uint32_t ctx, unsigned 
> > engine, uint32_t dep,
> >   
> >   spin->out_fence = emit_recursive_batch(spin, fd, ctx, engine, dep,
> >  out_fence);
> > - igt_assert(gem_bo_busy(fd, spin->handle));
> >   
> >   pthread_mutex_lock(&list_lock);
> >   igt_list_add(&spin->link, &spin_list);
> > @@ -238,9 +238,14 @@ __igt_spin_batch_new(int fd, uint32_t ctx, unsigned 
> > engine, uint32_t dep)
> >   igt_spin_t *
> >   igt_spin_batch_new(int fd, uint32_t ctx, unsigned engine, uint32_t dep)
> >   {
> > + igt_spin_t *spin;
> > +
> >   igt_require_gem(fd);
> >   
> > - return __igt_spin_batch_new(fd, ctx, engine, dep);
> > + spin = __igt_spin_batch_new(fd, ctx, engine, dep);
> > + igt_assert(gem_bo_busy(fd, spin->handle));
> 
> Should all uses of the low level constructors be changed to this high 
> level one if the test doesn't have the race with i915_wedged?

No. There is the waiting side-effect as well which is more often the
problem.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH igt] lib/dummyload: Avoid assertions in lowlevel spin constructor

[Antonio Argenziano]

On 22/02/18 00:50, Chris Wilson wrote:

__igt_spin_batch_new() may be used inside a background helper which is
competing against the GPU being reset. As such, we cannot even assert
that the spin->handle is busy immediately after submission as it may
have already been reset by another client writing to i915_wedged.

Signed-off-by: Chris Wilson 
---
  lib/igt_dummyload.c | 17 ++---
  tests/pm_rps.c  |  4 +++-
  2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/lib/igt_dummyload.c b/lib/igt_dummyload.c
index 27eb402b..8ecd1dc8 100644
--- a/lib/igt_dummyload.c
+++ b/lib/igt_dummyload.c
@@ -25,6 +25,7 @@
  #include 
  #include 
  #include 
+#include 
  
  #include 
  
@@ -205,7 +206,6 @@ ___igt_spin_batch_new(int fd, uint32_t ctx, unsigned engine, uint32_t dep,
  
  	spin->out_fence = emit_recursive_batch(spin, fd, ctx, engine, dep,

   out_fence);
-   igt_assert(gem_bo_busy(fd, spin->handle));
  
  	pthread_mutex_lock(&list_lock);

igt_list_add(&spin->link, &spin_list);
@@ -238,9 +238,14 @@ __igt_spin_batch_new(int fd, uint32_t ctx, unsigned 
engine, uint32_t dep)
  igt_spin_t *
  igt_spin_batch_new(int fd, uint32_t ctx, unsigned engine, uint32_t dep)
  {
+   igt_spin_t *spin;
+
igt_require_gem(fd);
  
-	return __igt_spin_batch_new(fd, ctx, engine, dep);

+   spin = __igt_spin_batch_new(fd, ctx, engine, dep);
+   igt_assert(gem_bo_busy(fd, spin->handle));


Should all uses of the low level constructors be changed to this high 
level one if the test doesn't have the race with i915_wedged?


Thanks,
Antonio


+
+   return spin;
  }
  
  igt_spin_t *

@@ -267,10 +272,16 @@ __igt_spin_batch_new_fence(int fd, uint32_t ctx, unsigned 
engine)
  igt_spin_t *
  igt_spin_batch_new_fence(int fd, uint32_t ctx, unsigned engine)
  {
+   igt_spin_t *spin;
+
igt_require_gem(fd);
igt_require(gem_has_exec_fence(fd));
  
-	return __igt_spin_batch_new_fence(fd, ctx, engine);

+   spin = __igt_spin_batch_new_fence(fd, ctx, engine);
+   igt_assert(gem_bo_busy(fd, spin->handle));
+   igt_assert(poll(&(struct pollfd){spin->out_fence, POLLIN}, 1, 0) == 0);
+
+   return spin;
  }
  
  static void notify(union sigval arg)

diff --git a/tests/pm_rps.c b/tests/pm_rps.c
index f71976ed..006d084b 100644
--- a/tests/pm_rps.c
+++ b/tests/pm_rps.c
@@ -221,6 +221,8 @@ static void load_helper_run(enum load load)
return;
}
  
+	igt_require_gem(drm_fd);

+
lh.exit = false;
lh.load = load;
  
@@ -233,7 +235,7 @@ static void load_helper_run(enum load load)
  
  		igt_debug("Applying %s load...\n", lh.load ? "high" : "low");
  
-		spin[0] = igt_spin_batch_new(drm_fd, 0, 0, 0);

+   spin[0] = __igt_spin_batch_new(drm_fd, 0, 0, 0);
if (lh.load == HIGH)
spin[1] = __igt_spin_batch_new(drm_fd, 0, 0, 0);
while (!lh.exit) {


___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx