On Thu, Jul 11, 2019 at 07:51:59AM +0100, Chris Wilson wrote:
> Right idea, wrong lock. We already drop struct_mutex before we free the
> mmap_offset when freeing the object, so we need to take the vma manager
> lock when manipulating the mmap_offset address space for our selftests.
> 
> Fixes: 8221d21b0664 ("drm/i915/selftests: Lock the drm_mm while modifying")
> Signed-off-by: Chris Wilson <[email protected]>
> Cc: Imre Deak <[email protected]>

Looks correct to me, obj->refcount=0 while its mmap_offset is still not
removed is a valid scenario and for mmap_offset lookup/changes vm_lock
should be enough:

Reviewed-by: Imre Deak <[email protected]>

> ---
>  .../drm/i915/gem/selftests/i915_gem_mman.c    | 20 +++++++++++++++----
>  1 file changed, 16 insertions(+), 4 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 b95fdc2b6bfc..5635cbb4af22 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/i915_gem_mman.c
> @@ -401,6 +401,18 @@ static void restore_retire_worker(struct 
> drm_i915_private *i915)
>       i915_gem_shrinker_register(i915);
>  }
>  
> +static void mmap_offset_lock(struct drm_i915_private *i915)
> +     __acquires(&i915->drm.vma_offset_manager->vm_lock)
> +{
> +     write_lock(&i915->drm.vma_offset_manager->vm_lock);
> +}
> +
> +static void mmap_offset_unlock(struct drm_i915_private *i915)
> +     __releases(&i915->drm.vma_offset_manager->vm_lock)
> +{
> +     write_unlock(&i915->drm.vma_offset_manager->vm_lock);
> +}
> +
>  static int igt_mmap_offset_exhaustion(void *arg)
>  {
>       struct drm_i915_private *i915 = arg;
> @@ -419,9 +431,9 @@ static int igt_mmap_offset_exhaustion(void *arg)
>       drm_mm_for_each_hole(hole, mm, hole_start, hole_end) {
>               resv.start = hole_start;
>               resv.size = hole_end - hole_start - 1; /* PAGE_SIZE units */
> -             mutex_lock(&i915->drm.struct_mutex);
> +             mmap_offset_lock(i915);
>               err = drm_mm_reserve_node(mm, &resv);
> -             mutex_unlock(&i915->drm.struct_mutex);
> +             mmap_offset_unlock(i915);
>               if (err) {
>                       pr_err("Failed to trim VMA manager, err=%d\n", err);
>                       goto out_park;
> @@ -485,9 +497,9 @@ static int igt_mmap_offset_exhaustion(void *arg)
>       }
>  
>  out:
> -     mutex_lock(&i915->drm.struct_mutex);
> +     mmap_offset_lock(i915);
>       drm_mm_remove_node(&resv);
> -     mutex_unlock(&i915->drm.struct_mutex);
> +     mmap_offset_unlock(i915);
>  out_park:
>       restore_retire_worker(i915);
>       return err;
> -- 
> 2.22.0
> 
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to