| Message ID | 20221004082047.23902-1-niranjana.vishwanathapura@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] drm/i915/selftests: Fix overwriting ggtt | expand |
On 04/10/2022 09:20, Niranjana Vishwanathapura wrote: > Looks like vma and gtt mock selftests overwrite the ggtt > initialized by mock_gem_device() by calling intel_gt_assign_ggtt() > and mock_init_ggtt() functions. > The side effect seems to be mock_destroy_device() tries to > release the ggtt already released by mock_fini_ggtt() instead of > releasing the ggtt created by mock_gem_device() function. > > I ran into this issue with vm_bind patch series. > https://patchwork.freedesktop.org/series/105879/#rev5 > > I am fixing the vm_bind patch series by remvoing the vm->root_obj > initialization for ggtt (it is required only for ppgtt upon user > request). > > But we need to address the underlying problem. > But with this patch, I am seeing some of the vma and gtt mock > selftests run into other crashes. So, this patch is not complete > and I am missing the big picture here. Hence this RFC. Adding some cc based on latest changes in the area being cdeea858d8df ("drm/i915: Remove unused i915->ggtt"). No implications intended just trying to help with brainstorming the problem. Regards, Tvrtko > Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com> > --- > drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 16 ++-------------- > drivers/gpu/drm/i915/selftests/i915_vma.c | 16 ++-------------- > 2 files changed, 4 insertions(+), 28 deletions(-) > > diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c > index ea2cf1080979..9b0dae1d1a68 100644 > --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c > @@ -1919,30 +1919,18 @@ int i915_gem_gtt_mock_selftests(void) > SUBTEST(igt_gtt_insert), > }; > struct drm_i915_private *i915; > - struct intel_gt *gt; > int err; > > i915 = mock_gem_device(); > if (!i915) > return -ENOMEM; > > - /* allocate the ggtt */ > - err = intel_gt_assign_ggtt(to_gt(i915)); > - if (err) > - goto out_put; > - > - gt = to_gt(i915); > - > - mock_init_ggtt(gt); > - > - err = i915_subtests(tests, gt->ggtt); > + err = i915_subtests(tests, to_gt(i915)->ggtt); > > mock_device_flush(i915); > i915_gem_drain_freed_objects(i915); > - mock_fini_ggtt(gt->ggtt); > - > -out_put: > mock_destroy_device(i915); > + > return err; > } > > diff --git a/drivers/gpu/drm/i915/selftests/i915_vma.c b/drivers/gpu/drm/i915/selftests/i915_vma.c > index 71b52d5efef4..dbe78ed408e5 100644 > --- a/drivers/gpu/drm/i915/selftests/i915_vma.c > +++ b/drivers/gpu/drm/i915/selftests/i915_vma.c > @@ -923,30 +923,18 @@ int i915_vma_mock_selftests(void) > SUBTEST(igt_vma_partial), > }; > struct drm_i915_private *i915; > - struct intel_gt *gt; > int err; > > i915 = mock_gem_device(); > if (!i915) > return -ENOMEM; > > - /* allocate the ggtt */ > - err = intel_gt_assign_ggtt(to_gt(i915)); > - if (err) > - goto out_put; > - > - gt = to_gt(i915); > - > - mock_init_ggtt(gt); > - > - err = i915_subtests(tests, gt->ggtt); > + err = i915_subtests(tests, to_gt(i915)->ggtt); > > mock_device_flush(i915); > i915_gem_drain_freed_objects(i915); > - mock_fini_ggtt(gt->ggtt); > - > -out_put: > mock_destroy_device(i915); > + > return err; > } >
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c index ea2cf1080979..9b0dae1d1a68 100644 --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c @@ -1919,30 +1919,18 @@ int i915_gem_gtt_mock_selftests(void) SUBTEST(igt_gtt_insert), }; struct drm_i915_private *i915; - struct intel_gt *gt; int err; i915 = mock_gem_device(); if (!i915) return -ENOMEM; - /* allocate the ggtt */ - err = intel_gt_assign_ggtt(to_gt(i915)); - if (err) - goto out_put; - - gt = to_gt(i915); - - mock_init_ggtt(gt); - - err = i915_subtests(tests, gt->ggtt); + err = i915_subtests(tests, to_gt(i915)->ggtt); mock_device_flush(i915); i915_gem_drain_freed_objects(i915); - mock_fini_ggtt(gt->ggtt); - -out_put: mock_destroy_device(i915); + return err; } diff --git a/drivers/gpu/drm/i915/selftests/i915_vma.c b/drivers/gpu/drm/i915/selftests/i915_vma.c index 71b52d5efef4..dbe78ed408e5 100644 --- a/drivers/gpu/drm/i915/selftests/i915_vma.c +++ b/drivers/gpu/drm/i915/selftests/i915_vma.c @@ -923,30 +923,18 @@ int i915_vma_mock_selftests(void) SUBTEST(igt_vma_partial), }; struct drm_i915_private *i915; - struct intel_gt *gt; int err; i915 = mock_gem_device(); if (!i915) return -ENOMEM; - /* allocate the ggtt */ - err = intel_gt_assign_ggtt(to_gt(i915)); - if (err) - goto out_put; - - gt = to_gt(i915); - - mock_init_ggtt(gt); - - err = i915_subtests(tests, gt->ggtt); + err = i915_subtests(tests, to_gt(i915)->ggtt); mock_device_flush(i915); i915_gem_drain_freed_objects(i915); - mock_fini_ggtt(gt->ggtt); - -out_put: mock_destroy_device(i915); + return err; }
Looks like vma and gtt mock selftests overwrite the ggtt initialized by mock_gem_device() by calling intel_gt_assign_ggtt() and mock_init_ggtt() functions. The side effect seems to be mock_destroy_device() tries to release the ggtt already released by mock_fini_ggtt() instead of releasing the ggtt created by mock_gem_device() function. I ran into this issue with vm_bind patch series. https://patchwork.freedesktop.org/series/105879/#rev5 I am fixing the vm_bind patch series by remvoing the vm->root_obj initialization for ggtt (it is required only for ppgtt upon user request). But we need to address the underlying problem. But with this patch, I am seeing some of the vma and gtt mock selftests run into other crashes. So, this patch is not complete and I am missing the big picture here. Hence this RFC. Signed-off-by: Niranjana Vishwanathapura <niranjana.vishwanathapura@intel.com> --- drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 16 ++-------------- drivers/gpu/drm/i915/selftests/i915_vma.c | 16 ++-------------- 2 files changed, 4 insertions(+), 28 deletions(-)