From: Peter Senna Tschudin <me@petersenna.com>
To: igt-dev@lists.freedesktop.org, kamil.konieczny@linux.intel.com,
andi.shyti@linux.intel.com
Subject: [PATCH i-g-t] tests/intel/gem_exec_capture: Fix many-* subtests
Date: Wed, 10 Apr 2024 19:40:22 +0200 [thread overview]
Message-ID: <20240410174031.5840-1-me@petersenna.com> (raw)
Currently trying to run `gem_exec_capture --run-subtest
many-4K-incremental` or `gem_exec_capture --run-subtest many-4K-zero`
will fail with:
(gem_exec_capture:81999) i915/gem_engine_topology-CRITICAL: Test
assertion failure function gem_engine_properties_configure, file
../lib/i915/gem_engine_topology.c:577:
(gem_exec_capture:81999) i915/gem_engine_topology-CRITICAL: Failed assertion: ret == 1
(gem_exec_capture:81999) i915/gem_engine_topology-CRITICAL: Last errno: 9, Bad file descriptor
(gem_exec_capture:81999) i915/gem_engine_topology-CRITICAL: error: -1 != 1
This problem happens inside the macro find_first_available_engine()
when:
1. for_each_ctx_engine() allocates an struct intel_engine_data 'ed'
inside a for loop. The core of the issue is that ed only exists
inside the for loop. As soon as the for loop ends, ed is out of scope
and after it's lifetime.
2. intel_get_current_engine() sets '*e' to an address of ed. This is ok
while inside the for loop, and is undefined behavior after the for
loop ends.
3. configure_hangs() uses '*e' after the lifetime of 'ed' has ended
leading to undefined behavior
4. After the call to find_first_available_engine() __captureN() will
fail as it expects '*e' to be valid. This is also undefined
behavior.
Using 'e' outside for_each_ctx_engine() is incorrect and this patch
fixes it. Additionally, this patch moves the 'igt_assert(tmpe);' to
between the assignment and first use of 'tmpe' to prevent code
from trying to use a NULL 'tmpe'.
Reviewed-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
Signed-off-by: Peter Senna Tschudin <me@petersenna.com>
---
Sending as a new patch instead of v8 of the following series:
https://patchwork.freedesktop.org/series/131602/
tests/intel/gem_exec_capture.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/tests/intel/gem_exec_capture.c b/tests/intel/gem_exec_capture.c
index 57b178f3e..8800fa586 100644
--- a/tests/intel/gem_exec_capture.c
+++ b/tests/intel/gem_exec_capture.c
@@ -662,13 +662,20 @@ static bool needs_recoverable_ctx(int fd)
#define find_first_available_engine(fd, ctx, e, saved) \
do { \
+ struct intel_execution_engine2 *tmpe = NULL; \
+ e = NULL; \
+ \
ctx = intel_ctx_create_all_physical(fd); \
igt_assert(ctx); \
- for_each_ctx_engine(fd, ctx, e) \
- for_each_if(gem_class_can_store_dword(fd, e->class)) \
+ for_each_ctx_engine(fd, ctx, tmpe) { \
+ igt_assert(tmpe); \
+ if(gem_class_can_store_dword(fd, tmpe->class)) { \
+ saved = configure_hangs(fd, tmpe, ctx->id); \
+ e = &saved.engine; \
break; \
- igt_assert(e); \
- saved = configure_hangs(fd, e, ctx->id); \
+ } \
+ } \
+ igt_skip_on_f(e == NULL, "no capable engine found\n"); \
} while(0)
static void many(int fd, int dir, uint64_t size, unsigned int flags)
--
2.44.0
next reply other threads:[~2024-04-10 17:40 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-10 17:40 Peter Senna Tschudin [this message]
2024-04-11 1:05 ` ✓ CI.xeBAT: success for tests/intel/gem_exec_capture: Fix many-* subtests Patchwork
2024-04-11 1:11 ` ✓ Fi.CI.BAT: " Patchwork
2024-04-11 12:09 ` [PATCH i-g-t] " Kamil Konieczny
2024-04-11 12:13 ` ✗ CI.xeFULL: failure for " Patchwork
2024-04-11 14:20 ` ✗ Fi.CI.IGT: " Patchwork
2024-04-11 19:53 ` [PATCH i-g-t v2] " Peter Senna Tschudin
2024-04-19 15:05 ` Kamil Konieczny
2024-04-23 4:15 ` Peter Senna Tschudin
2024-04-11 20:52 ` ✓ CI.xeBAT: success for tests/intel/gem_exec_capture: Fix many-* subtests (rev2) Patchwork
2024-04-11 21:00 ` ✗ Fi.CI.BAT: failure " Patchwork
2024-04-19 14:27 ` Kamil Konieczny
2024-04-12 4:24 ` ✗ CI.xeFULL: " Patchwork
2024-04-22 15:24 ` ✓ CI.xeBAT: success for tests/intel/gem_exec_capture: Fix many-* subtests (rev3) Patchwork
2024-04-22 15:25 ` ✓ Fi.CI.BAT: " Patchwork
2024-04-22 20:48 ` ✗ Fi.CI.IGT: failure " Patchwork
2024-04-22 21:48 ` ✗ CI.xeFULL: " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240410174031.5840-1-me@petersenna.com \
--to=me@petersenna.com \
--cc=andi.shyti@linux.intel.com \
--cc=igt-dev@lists.freedesktop.org \
--cc=kamil.konieczny@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox