| Message ID | 20180512090342.6431-1-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 12/05/18 02:03, Chris Wilson wrote: > If we trigger "too many" resets, the context and even the file, will be > banend and subsequent execbufs should fail with -EIO. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Does this replace gem_reset_stats@test_ban? Thanks, Antonio
Quoting Antonio Argenziano (2018-05-14 15:51:04) > > > On 12/05/18 02:03, Chris Wilson wrote: > > If we trigger "too many" resets, the context and even the file, will be > > banend and subsequent execbufs should fail with -EIO. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Does this replace gem_reset_stats@test_ban? gem_reset_stats was queued to be rewritten from scratch a few years ago. In short, no it doesn't replace as they are asking slightly different questions. gem_eio is asking that if banned we get EIO. I have no idea what API gem_reset_stats is supposed to be asking about, since banning is not an aspect of DRM_IOCTL_I915_GET_RESET_STATS and so should be treated very lightly to avoid over-specificity. (Banning is an internal kernel policy in the name of DoS prevention and not a rigorous defense or subject to user control.) -Chris
On 14/05/18 08:02, Chris Wilson wrote: > Quoting Antonio Argenziano (2018-05-14 15:51:04) >> >> >> On 12/05/18 02:03, Chris Wilson wrote: >>> If we trigger "too many" resets, the context and even the file, will be >>> banend and subsequent execbufs should fail with -EIO. >>> >>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> >> >> Does this replace gem_reset_stats@test_ban? > > gem_reset_stats was queued to be rewritten from scratch a few years ago. > > In short, no it doesn't replace as they are asking slightly different > questions. gem_eio is asking that if banned we get EIO. I have no idea > what API gem_reset_stats is supposed to be asking about, since banning > is not an aspect of DRM_IOCTL_I915_GET_RESET_STATS and so should be > treated very lightly to avoid over-specificity. (Banning is an internal > kernel policy in the name of DoS prevention and not a rigorous defense > or subject to user control.) I am not sure how much the intention of the tests are different :), but if that is the case then we need to check that other contexts are not being affected after a ban and they do not report -EIO on submission. Thanks, Antonio > -Chris >
diff --git a/tests/gem_eio.c b/tests/gem_eio.c index 6455c6acd..4720b47b5 100644 --- a/tests/gem_eio.c +++ b/tests/gem_eio.c @@ -250,6 +250,52 @@ static int __check_wait(int fd, uint32_t bo, unsigned int wait) return ret; } +static void __test_banned(int fd) +{ + struct drm_i915_gem_exec_object2 obj = { + .handle = gem_create(fd, 4096), + }; + struct drm_i915_gem_execbuffer2 execbuf = { + .buffers_ptr = to_user_pointer(&obj), + .buffer_count = 1, + }; + const uint32_t bbe = MI_BATCH_BUFFER_END; + unsigned long count = 0; + + gem_write(fd, obj.handle, 0, &bbe, sizeof(bbe)); + + gem_quiescent_gpu(fd); + igt_require(i915_reset_control(true)); + + igt_until_timeout(5) { + igt_spin_t *hang; + + if (__gem_execbuf(fd, &execbuf) == -EIO) { + igt_info("Banned after causing %lu hangs\n", count); + igt_assert(count > 1); + return; + } + + /* Trigger a reset, making sure we are detected as guilty */ + hang = spin_sync(fd, 0, 0); + trigger_reset(fd); + igt_spin_batch_free(fd, hang); + + count++; + } + + igt_assert_f(false, + "Ran for 5s, %lu hangs without being banned\n", + count); +} + +static void test_banned(int fd) +{ + fd = gem_reopen_driver(fd); + __test_banned(fd); + close(fd); +} + #define TEST_WEDGE (1) static void test_wait(int fd, unsigned int flags, unsigned int wait) @@ -693,6 +739,9 @@ igt_main igt_subtest("execbuf") test_execbuf(fd); + igt_subtest("banned") + test_banned(fd); + igt_subtest("suspend") test_suspend(fd, SUSPEND_STATE_MEM);
If we trigger "too many" resets, the context and even the file, will be banend and subsequent execbufs should fail with -EIO. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- tests/gem_eio.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+)