Re: [igt-dev] [RFC PATCH i-g-t v4 1/4] tests/gem_exec_reloc: Don't filter out invalid addresses

Chris Wilson
2019-11-01T10:02:45Z

Quoting Janusz Krzysztofik (2019-10-31 15:28:54)
> Commit a355b2d6eb42 ("igt/gem_exec_reloc: Filter out unavailable
> addresses for !ppgtt") introduced filtering of addresses possibly
> occupied by other users of shared GTT. Unfortunately, that filtering
> doesn't distinguish between actually occupied addresses and otherwise
> invalid softpin offsets. As soon as incorrect GTT alignment is assumed
> when running on future backends with possibly larger minimum page
> sizes, a half of calculated offsets to be tested will be incorrectly
> detected as occupied by other users and silently skipped instead of
> reported as a problem. That will significantly distort the intended
> test pattern.
>
> Filter out failing addresses only if not reported as invalid.
>
> v2: Skip unavailable addresses only when not running on full PPGTT.
> v3: Replace the not on full PPGTT requirement for skipping with error
> code checking.
>
> Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> tests/i915/gem_exec_reloc.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/tests/i915/gem_exec_reloc.c b/tests/i915/gem_exec_reloc.c
> index 5f59fe99..423fed8b 100644
> --- a/tests/i915/gem_exec_reloc.c
> +++ b/tests/i915/gem_exec_reloc.c
> @@ -520,7 +520,7 @@ static void basic_range(int fd, unsigned flags)
> uint64_t gtt_size = gem_aperture_size(fd);
> const uint32_t bbe = MI_BATCH_BUFFER_END;
> igt_spin_t *spin = NULL;
> - int count, n;
> + int count, n, err;
>
> igt_require(gem_has_softpin(fd));
>
> @@ -542,8 +542,11 @@ static void basic_range(int fd, unsigned flags)
> gem_write(fd, obj[n].handle, 0, &bbe, sizeof(bbe));
> execbuf.buffers_ptr = to_user_pointer(&obj[n]);
> execbuf.buffer_count = 1;
> - if (__gem_execbuf(fd, &execbuf))
> + err = __gem_execbuf(fd, &execbuf);
> + if (err) {

> + igt_assert(err != -EINVAL);

I've been thinking about this and I think the right approach is

/* Iff using a shared GTT, some of it may be reserved */
igt_assert_eq(err, -ENOSPC);

> continue;
> + }