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

From: Vanshidhar Konda <vanshidhar.r.konda@intel.com>
To: Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org, igt-dev@lists.freedesktop.org
Subject: Re: [RFC PATCH i-g-t v4 1/4] tests/gem_exec_reloc: Don't filter out invalid addresses
Date: Thu, 31 Oct 2019 09:59:50 -0700	[thread overview]
Message-ID: <20191031165950.jytjykldypucxksd@vrkonda-desk.ra.intel.com> (raw)
In-Reply-To: <20191031152857.17143-2-janusz.krzysztofik@linux.intel.com>

May be this patch can just be merged with the other patch in this series
that changes gem_exec_reloc.

Vanshi

On Thu, Oct 31, 2019 at 04:28:54PM +0100, Janusz Krzysztofik wrote:
>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);
> 			continue;
>+		}
>
> 		igt_debug("obj[%d] handle=%d, address=%llx\n",
> 			  n, obj[n].handle, (long long)obj[n].offset);
>@@ -562,8 +565,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);
> 			continue;
>+		}
>
> 		igt_debug("obj[%d] handle=%d, address=%llx\n",
> 			  n, obj[n].handle, (long long)obj[n].offset);
>-- 
>2.21.0
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx