[igt-dev] [PATCH i-g-t] i915/gem_madvise: fix dontneed-*-mmap subtests

[Cavitt, Jonathan]

-----Original Message-----
From: Kamil Konieczny <kamil.konieczny at linux.intel.com> 
Sent: Friday, June 30, 2023 10:25 AM
To: Cavitt, Jonathan <jonathan.cavitt at intel.com>
Subject: Fw: [PATCH i-g-t] i915/gem_madvise: fix dontneed-*-mmap subtests
>
>Hi Jonathan,
>----- Forwarded message from Kamil Konieczny <kamil.konieczny at linux.intel.com> -----
>
>Date: Wed, 28 Jun 2023 10:45:28 +0200
>From: Kamil Konieczny <kamil.konieczny at linux.intel.com>
>To: igt-dev at lists.freedesktop.org
>Cc: Kamil Konieczny <kamil.konieczny at linux.intel.com>
>Subject: [PATCH i-g-t] i915/gem_madvise: fix dontneed-*-mmap subtests
>Message-Id: <20230628084528.18779-1-kamil.konieczny at linux.intel.com>
>X-Mailer: git-send-email 2.39.2
>
>Introduction of drm_close_driver() and following changes which
>uses it caught a bug in two dontneed subtests:
>
>Starting subtest: dontneed-after-mmap
>(gem_madvise:1464) drmtest-WARNING: Don't attempt to close standard/invalid file descriptor: 5
>
>Starting subtest: dontneed-before-mmap
>(gem_madvise:1708) drmtest-WARNING: Don't attempt to close standard/invalid file descriptor: 5
>
>It turns out that closing fd was done regardless of mmap
>failure or success, that leads to stopping testing due to closed
>fd and later at test exit or loop iteration there is attempt to
>close already closed fd.
>
>Fixed that by doing close just before actual testing. Also while
>at this, fix style and remove one unnecessery empty line.
>
>Signed-off-by: Kamil Konieczny <kamil.konieczny at linux.intel.com>

Reviewed-by: Jonathan Cavitt <jonathan.cavitt at intel.com>

I'm also forwarding this to igt-dev because they need to know I'm RBing this for bookkeeping reasons.  That's why I couldn't just give my RB in Teams.
-Jonathan Cavitt

>---
> tests/i915/gem_madvise.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
>diff --git a/tests/i915/gem_madvise.c b/tests/i915/gem_madvise.c
>index 765809e7f..06b3ddf32 100644
>--- a/tests/i915/gem_madvise.c
>+++ b/tests/i915/gem_madvise.c
>@@ -109,11 +109,10 @@ dontneed_before_mmap(void)
> 		ptr = __gem_mmap_offset(fd, handle, 0, OBJECT_SIZE,
> 					PROT_READ | PROT_WRITE,
> 					t->type);
>-
>-		drm_close_driver(fd);
> 		if (!ptr)
> 			continue;
> 
>+		drm_close_driver(fd);
> 		old_sigsegv = signal(SIGSEGV, sigtrap);
> 		old_sigbus = signal(SIGBUS, sigtrap);
> 		switch (sigsetjmp(jmp, SIGBUS | SIGSEGV)) {
>@@ -156,10 +155,10 @@ dontneed_after_mmap(void)
> 					t->type);
> 
> 		gem_madvise(fd, handle, I915_MADV_DONTNEED);
>-		drm_close_driver(fd);
> 		if (!ptr)
> 			continue;
> 
>+		drm_close_driver(fd);
> 		old_sigsegv = signal(SIGSEGV, sigtrap);
> 		old_sigbus = signal(SIGBUS, sigtrap);
> 		switch (sigsetjmp(jmp, SIGBUS | SIGSEGV)) {
>-- 
>2.39.2
>
>
>----- End forwarded message -----
>