On Tue Jun 10, 2025 at 10:21 AM UTC, Krzysztof Karas wrote: > 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 <krzysztof.ka...@intel.com> > --- > > 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); I'll try to avoid calling fput() here and instead handle it where the file is created. This will simplify tracking the file's lifecycle.
-- Best regards, Sebastian
