[igt-dev] [PATCH i-g-t v2] lib/hang_ctx: Make use of dummyload library to create recursive batch

[Chris Wilson]

Quoting Antonio Argenziano (2018-07-10 16:31:55)
> An hanging batch is nothing more than a spinning batch that never gets
> stopped, so re-use the routines implemented in dummyload.c.
> 
> v2:
>         - Let caller decide spin loop size (Chris)
>         - Now builds with meson.
> 
> Signed-off-by: Antonio Argenziano <antonio.argenziano at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  lib/igt_gt.c                 | 65 +++++++++-----------------------------------
>  lib/igt_gt.h                 |  3 +-
>  lib/meson.build              |  1 +
>  tests/gem_concurrent_all.c   |  6 ++--
>  tests/gem_ctx_exec.c         |  2 +-
>  tests/gem_mmap_gtt.c         |  2 +-
>  tests/gem_pread_after_blit.c |  2 +-
>  tests/gem_reloc_vs_gpu.c     |  2 +-
>  tests/gem_ringfill.c         |  2 +-
>  tests/gem_shrink.c           |  2 +-
>  tests/kms_flip.c             |  2 +-
>  tests/kms_vblank.c           |  2 +-
>  12 files changed, 27 insertions(+), 64 deletions(-)
> 
> diff --git a/lib/igt_gt.c b/lib/igt_gt.c
> index 4569fd36..4ca4beef 100644
> --- a/lib/igt_gt.c
> +++ b/lib/igt_gt.c
> @@ -40,6 +40,7 @@
>  #include "ioctl_wrappers.h"
>  #include "intel_reg.h"
>  #include "intel_chipset.h"
> +#include "igt_dummyload.h"
>  
>  /**
>   * SECTION:igt_gt
> @@ -264,14 +265,10 @@ igt_hang_t igt_hang_ctx(int fd,
>                         unsigned flags,
>                         uint64_t *offset)
>  {
> -       struct drm_i915_gem_relocation_entry reloc;
> -       struct drm_i915_gem_execbuffer2 execbuf;
> -       struct drm_i915_gem_exec_object2 exec;
> +       igt_spin_t *spinning_batch;
> +       struct igt_spin_factory opts = {};
>         struct drm_i915_gem_context_param param;
> -       uint32_t b[16];
>         unsigned ban;
> -       unsigned len;
> -       int gen;
>  
>         igt_require_hang_ring(fd, ring);
>  
> @@ -295,58 +292,22 @@ igt_hang_t igt_hang_ctx(int fd,
>         if ((flags & HANG_ALLOW_BAN) == 0)
>                 context_set_ban(fd, ctx, 0);
>  
> -       memset(&reloc, 0, sizeof(reloc));
> -       memset(&exec, 0, sizeof(exec));
> -       memset(&execbuf, 0, sizeof(execbuf));
> -
> -       exec.handle = gem_create(fd, 4096);
> -       exec.relocation_count = 1;
> -       exec.relocs_ptr = to_user_pointer(&reloc);
> -
> -       memset(b, 0xc5, sizeof(b));
> -
> -       len = 0;
> -       gen = intel_gen(intel_get_drm_devid(fd));
> -       if (gen >= 8) {
> -               b[len++] = MI_BATCH_BUFFER_START | 1 << 8 | 1;
> -               b[len++] = 0;
> -               b[len++] = 0;
> -       } else if (gen >= 6) {
> -               b[len++] = MI_BATCH_BUFFER_START | 1 << 8;
> -               b[len++] = 0;
> -       } else {
> -               b[len++] = MI_BATCH_BUFFER_START | 2 << 6;
> -               b[len] = 0;
> -               if (gen < 4) {
> -                       b[len] |= 1;
> -                       reloc.delta = 1;
> -               }
> -               len++;
> -       }
> -       b[len++] = MI_BATCH_BUFFER_END;
> -       b[len] = MI_NOOP;
> -       gem_write(fd, exec.handle, 0, b, sizeof(b));
> -
> -       reloc.offset = sizeof(uint32_t);
> -       reloc.target_handle = exec.handle;
> -       reloc.read_domains = I915_GEM_DOMAIN_COMMAND;
> -
> -       execbuf.buffers_ptr = to_user_pointer(&exec);
> -       execbuf.buffer_count = 1;
> -       execbuf.flags = ring;
> -       i915_execbuffer2_set_context_id(execbuf, ctx);
> -       gem_execbuf(fd, &execbuf);
> +       opts.ctx = ctx;
> +       opts.engine = ring;
> +       opts.flags |= (flags & HANG_SPIN_FAST) ? IGT_SPIN_FAST : 0; /* hangs faster */
> +       spinning_batch = igt_spin_batch_factory(fd, &opts);
>  
>         if (offset)
> -               *offset = exec.offset;
> +               *offset = (*spinning_batch).obj[1].offset; /* The batch is the last object */

K&R came up with this shorthand I think you should use, ->

> diff --git a/lib/igt_gt.h b/lib/igt_gt.h
> index d44b7552..12a83a1b 100644
> --- a/lib/igt_gt.h
> +++ b/lib/igt_gt.h
> @@ -50,8 +50,9 @@ igt_hang_t igt_hang_ctx(int fd,
>                         uint64_t *offset);
>  #define HANG_ALLOW_BAN 1
>  #define HANG_ALLOW_CAPTURE 2
> +#define HANG_SPIN_FAST 3

Must be a power-of-two, or wrap in BIT() which doesn't exist yet...

Overall I am still dubious there is any reason to set this flag. The
ratelimiting step here isn't the speed at which the batch spins, but the
time for hangcheck to kick in. Where a fast spinner is interesting is
the latency in responding to terminating the spinner (which never
happens for a hang!).
-Chris