igt_mmap_migrate() tests migration with various parameters.
In one of the cases, where FILL and UNFAULTABLE flags are set,
during first stages of this test a mock file is opened in
igt_mmap_offset(), which results in allocating some data in the
GPU memory. Then, also in igt_mmap_offset(), the file is closed
(fput) and the cleanup of that data is scheduled. Right after
that, the test calls igt_fill_mappable() to fill all available
GPU memory. At this point, three scenarios are possible
(N = max size of GPU memory for this test in MiB):
 1) the data cleanup does not fire until the whole test is over,
so the memory is fully occupied by 1 MiB with that data and
N - 1 MiB added by igt_fill_mappable(),
 2) the data cleanup fires before igt_fill_mappable() completes,
so the whole memory is populated with N MiB by
igt_fill_mappable(),
 3) the data cleanup is performed right after fill is done,
so only N - 1 MiB are in the GPU memory, preventing the test
from properly faulting - we'd expect no space left, but an
object was able to fit in the remaining 1 MiB.

Amend the problem by keeping the mock file open throughout the
duration of this test and calling fput() from the test itself.

Closes: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13929
Signed-off-by: Krzysztof Karas <[email protected]>
---

On DG2 platforms the memory for data allocated as a result of
opening a mock file remains occupied until the test is done
(scenario 1), but on ATSM cards the data is freed earlier
(scenarios 2 and 3), which leads to sporadic failures.

During testing I observed that the max memory was equal
to either 4096 or 2048 and igt_fill_mappable() tries to allocate
as many 1k objects as possible before halving allocation size.

 .../drm/i915/gem/selftests/i915_gem_mman.c    |  6 ++-
 drivers/gpu/drm/i915/selftests/igt_mmap.c     | 54 +++++++++++++------
 drivers/gpu/drm/i915/selftests/igt_mmap.h     |  8 +++
 3 files changed, 51 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c 
b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
index 9c3f17e51885..1fe4a45d3efb 100644
--- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
@@ -1176,6 +1176,7 @@ static int __igt_mmap_migrate(struct intel_memory_region 
**placements,
        struct drm_i915_private *i915 = placements[0]->i915;
        struct drm_i915_gem_object *obj;
        struct i915_request *rq = NULL;
+       struct file *mock_file;
        unsigned long addr;
        LIST_HEAD(objects);
        u64 offset;
@@ -1200,8 +1201,8 @@ static int __igt_mmap_migrate(struct intel_memory_region 
**placements,
         * level paging structures(and perhaps scratch), so make sure we
         * allocate early, to avoid tears.
         */
-       addr = igt_mmap_offset(i915, offset, obj->base.size,
-                              PROT_WRITE, MAP_SHARED);
+       addr = igt_mmap_offset_get_file(i915, offset, obj->base.size,
+                                       PROT_WRITE, MAP_SHARED, &mock_file);
        if (IS_ERR_VALUE(addr)) {
                err = addr;
                goto out_put;
@@ -1299,6 +1300,7 @@ static int __igt_mmap_migrate(struct intel_memory_region 
**placements,
        }
 
 out_put:
+       fput(mock_file);
        i915_gem_object_put(obj);
        igt_close_objects(i915, &objects);
        return err;
diff --git a/drivers/gpu/drm/i915/selftests/igt_mmap.c 
b/drivers/gpu/drm/i915/selftests/igt_mmap.c
index e920a461bd36..237ad91cd009 100644
--- a/drivers/gpu/drm/i915/selftests/igt_mmap.c
+++ b/drivers/gpu/drm/i915/selftests/igt_mmap.c
@@ -9,14 +9,14 @@
 #include "i915_drv.h"
 #include "igt_mmap.h"
 
-unsigned long igt_mmap_offset(struct drm_i915_private *i915,
-                             u64 offset,
-                             unsigned long size,
-                             unsigned long prot,
-                             unsigned long flags)
+static unsigned long __igt_mmap_offset(struct drm_i915_private *i915,
+                                      u64 offset,
+                                      unsigned long size,
+                                      unsigned long prot,
+                                      unsigned long flags,
+                                      struct file **file)
 {
        struct drm_vma_offset_node *node;
-       struct file *file;
        unsigned long addr;
        int err;
 
@@ -32,21 +32,45 @@ unsigned long igt_mmap_offset(struct drm_i915_private *i915,
        }
 
        /* Pretend to open("/dev/dri/card0") */
-       file = mock_drm_getfile(i915->drm.primary, O_RDWR);
-       if (IS_ERR(file))
-               return PTR_ERR(file);
+       *file = mock_drm_getfile(i915->drm.primary, O_RDWR);
+       if (IS_ERR(*file))
+               return PTR_ERR(*file);
 
-       err = drm_vma_node_allow(node, file->private_data);
+       err = drm_vma_node_allow(node, (*file)->private_data);
        if (err) {
-               addr = err;
-               goto out_file;
+               fput(*file);
+               return err;
        }
 
-       addr = vm_mmap(file, 0, drm_vma_node_size(node) << PAGE_SHIFT,
+       addr = vm_mmap(*file, 0, drm_vma_node_size(node) << PAGE_SHIFT,
                       prot, flags, drm_vma_node_offset_addr(node));
 
-       drm_vma_node_revoke(node, file->private_data);
-out_file:
+       drm_vma_node_revoke(node, (*file)->private_data);
+
+       return addr;
+}
+
+unsigned long igt_mmap_offset(struct drm_i915_private *i915,
+                             u64 offset,
+                             unsigned long size,
+                             unsigned long prot,
+                             unsigned long flags)
+{
+       struct file *file;
+       unsigned long addr;
+
+       addr = __igt_mmap_offset(i915, offset, size, prot, flags, &file);
        fput(file);
+
        return addr;
 }
+
+unsigned long igt_mmap_offset_get_file(struct drm_i915_private *i915,
+                                      u64 offset,
+                                      unsigned long size,
+                                      unsigned long prot,
+                                      unsigned long flags,
+                                      struct file **file)
+{
+       return __igt_mmap_offset(i915, offset, size, prot, flags, file);
+}
diff --git a/drivers/gpu/drm/i915/selftests/igt_mmap.h 
b/drivers/gpu/drm/i915/selftests/igt_mmap.h
index acbe34d81a6d..756ccdf6fd69 100644
--- a/drivers/gpu/drm/i915/selftests/igt_mmap.h
+++ b/drivers/gpu/drm/i915/selftests/igt_mmap.h
@@ -11,6 +11,7 @@
 
 struct drm_i915_private;
 struct drm_vma_offset_node;
+struct file;
 
 unsigned long igt_mmap_offset(struct drm_i915_private *i915,
                              u64 offset,
@@ -18,4 +19,11 @@ unsigned long igt_mmap_offset(struct drm_i915_private *i915,
                              unsigned long prot,
                              unsigned long flags);
 
+unsigned long igt_mmap_offset_get_file(struct drm_i915_private *i915,
+                                      u64 offset,
+                                      unsigned long size,
+                                      unsigned long prot,
+                                      unsigned long flags,
+                                      struct file **file);
+
 #endif /* IGT_MMAP_H */
-- 
2.43.0

Reply via email to