A currently unavoidable lockdep loop related to userptr MMU notifier
exists inside the i915 driver. For as long as that issue is not
resolved, operations which are believed to potentially result in the
loop being triggered are expected to fail early to prevent from that
badness to happen.
The lockdep loop occurrence is believed to be possible for a userptr
object created on top of any type of mmap-offset mapping. For that to
happen, userptr MMU notifier, which is the source of the lockdep issue,
must be activated for the object. That is possible only if the object
has been created without I915_USERPTR_UNSYNCHRONIZED flag.
Attempts to create a userptr object on top of a mmap-offset mapping to
another GEM object currently always succeed. However, the very first
operation which tries to set the object pages fails with -EAGAIN. If
I915_USERPTR_UNSYNCHRONIZED flag has been used by object creation, MMU
notifier is activated for the object. Consecutive attempts may also
fail with -EAGAIN while the driver tries to acquire the pages in
background, with the MMU notifier still possibly active, but meanwhile,
the background attempt to pin the pages in memory finally fails, the
notifier is deactivated, and all following set pages operations fail
with -EFAULT.
There is a subtests which already exercises the driver behavior for
userptr objects created on top of mmap-offset mappings. However, the
exercise is performed on userptr objects created only with the
I915_USERPTR_UNSYNCHRONIZED flag set, then, the MMU notifier is never
activated.
Clone the subtest to a "-sync" variant so objects created without the
I915_USERPTR_UNSYNCHRONIZED flag are also exercised. In that case, try
to anger lockdep, but since that seems hardly possible, also display a
warning for as long as we believe the lockdep splat is possible in that
scenario. Also, don't fail but skip should the driver ever refuse to
create synchronized userptr objects on top of invalid mappings.
v2: For as long as the lockdep loop issue is not fixed, don't succeed
if a preventive failure occurs but skip (Chris),
- otherwise, warn about possible risk,
- put a FIXME placeholder until we learn how to anger lockdep.
v3: Use dynamic subtests, with skips handled at mmap-offset attempt
performed by the test anyway (Chris),
- for better clarity of the patch, drop cosmetic only changes,
- use more concise wording in subtest description.
v4: Limit the scope to lockdep loop trigger attempts, separate from
changes aimed at extending subtest coverage over new mapping types,
- as lockdep loop can happen only for userptr objects created without
I915_USERPTR_UNSYNCHRONIZED flag, introduce a new "-sync" variant
of the subtest which examines userptr objects created with the flag
not set,
- move lockdep loop trigger attempt to a separate function and call
it only when the I915_USERPTR_UNSYNCHRONIZED flag is not set,
- actually try to anger lockdep using gem_set_tiling() (Chris).
Suggested-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx
Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@xxxxxxxxxxxxxxx>
---
tests/i915/gem_userptr_blits.c | 53 ++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/tests/i915/gem_userptr_blits.c b/tests/i915/gem_userptr_blits.c
index 08015586a..c1496f245 100644
--- a/tests/i915/gem_userptr_blits.c
+++ b/tests/i915/gem_userptr_blits.c
@@ -577,6 +577,49 @@ static int test_invalid_null_pointer(int fd)
return 0;
}
+static void anger_lockdep(int fd, uint32_t parent_handle,
+ uint64_t parent_offset, const struct mmap_offset *t)
+{
+ struct drm_i915_gem_set_domain set_domain;
+ void *ptr;
+ uint32_t handle;
+
+ /* create the map */
+ ptr = mmap(NULL, PAGE_SIZE, PROT_READ | PROT_WRITE,
+ MAP_SHARED | MAP_FIXED, fd, parent_offset);
+ igt_assert(ptr != MAP_FAILED);
+ igt_assert(((unsigned long)ptr & (PAGE_SIZE - 1)) == 0);
+
+ /* create userptr */
+ igt_skip_on_f(__gem_userptr(fd, ptr, PAGE_SIZE, 0, userptr_flags,
+ &handle),
+ "userptr on mmap-offset(%s) banned, lockdep loop prevention\n",
+ t->name);
+
+ /* FIXME: learn how to actually anger lockdep reproducibly */
+ igt_warn("userptr on mmap_offset(%s) succeeded, risk of lockdep loop exists\n",
+ t->name);
+
+ memset(&set_domain, 0, sizeof(set_domain));
+ set_domain.handle = handle;
+ set_domain.read_domains = I915_GEM_DOMAIN_GTT;
+ set_domain.write_domain = I915_GEM_DOMAIN_GTT;
+
+ if (gem_available_fences(fd)) {
+ /* set object pages once so the MMU notifier is activated */
+ igt_assert(ioctl(fd, DRM_IOCTL_I915_GEM_SET_DOMAIN, &set_domain)
+ && errno == EAGAIN);
+ /* immediately try to trigger the notifier (likely too late) */
+ gem_set_tiling(fd, parent_handle, I915_TILING_Y, 512 * 4);
+ } else {
+ ; /* FIXME: find another way to invalidate parent pages */
+ }
+
+ /* cleanup */
+ gem_close(fd, handle);
+ munmap(ptr, sizeof(linear));
+}
+
static int test_invalid_mapping(int fd, const struct mmap_offset *t)
{
struct drm_i915_gem_mmap_offset arg;
@@ -608,6 +651,10 @@ static int test_invalid_mapping(int fd, const struct mmap_offset *t)
arg.flags = t->type;
igt_skip_on_f(igt_ioctl(fd, DRM_IOCTL_I915_GEM_MMAP_OFFSET, &arg),
"HW & kernel support for mmap_offset(%s)\n", t->name);
+
+ if (!(userptr_flags & LOCAL_I915_USERPTR_UNSYNCHRONIZED))
+ anger_lockdep(fd, arg.handle, arg.offset, t);
+
ptr = mmap(map + PAGE_SIZE, sizeof(linear), PROT_READ | PROT_WRITE,
MAP_SHARED | MAP_FIXED, fd, arg.offset);
igt_assert(ptr == map + PAGE_SIZE);
@@ -2161,6 +2208,12 @@ igt_main_args("c:", NULL, help_str, opt_handler, NULL)
count = intel_get_total_ram_mb() * 3 / 4;
}
+ igt_describe("Verify synchronized userptr on mmap-offset mappings fails");
+ igt_subtest_with_dynamic("invalid-mmap-offset-sync")
+ for_each_mmap_offset_type(fd, t)
+ igt_dynamic_f("%s", t->name)
+ test_invalid_mapping(fd, t);
+
igt_subtest("process-exit")
test_process_exit(fd, 0);
--
2.21.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
