Re: [Intel-gfx] [PATCH 01/23] drm/i915: Drop vm.ref for duplicate vma on construction

[Andi Shyti <andi@xxxxxxxxxxx>]

Hi Chris,

> diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
> index 1f63c4a1f055..7fe1f317cd2b 100644
> --- a/drivers/gpu/drm/i915/i915_vma.c
> +++ b/drivers/gpu/drm/i915/i915_vma.c
> @@ -198,6 +198,7 @@ vma_create(struct drm_i915_gem_object *obj,
>  		cmp = i915_vma_compare(pos, vm, view);
>  		if (cmp == 0) {
>  			spin_unlock(&obj->vma.lock);
> +			i915_vm_put(vm);
>  			i915_vma_free(vma);

You are forgettin one return without dereferencing it.

would this be a solution:

@@ -106,6 +106,7 @@ vma_create(struct drm_i915_gem_object *obj,
 {
        struct i915_vma *vma;
        struct rb_node *rb, **p;
+       struct i915_vma *pos = ERR_PTR(-E2BIG);
 
        /* The aliasing_ppgtt should never be used directly! */
        GEM_BUG_ON(vm == &vm->gt->ggtt->alias->vm);
@@ -185,7 +186,6 @@ vma_create(struct drm_i915_gem_object *obj,
        rb = NULL;
        p = &obj->vma.tree.rb_node;
        while (*p) {
-               struct i915_vma *pos;
                long cmp;
 
                rb = *p;
@@ -197,12 +197,8 @@ vma_create(struct drm_i915_gem_object *obj,
                 * and dispose of ours.
                 */
                cmp = i915_vma_compare(pos, vm, view);
-               if (cmp == 0) {
-                       spin_unlock(&obj->vma.lock);
-                       i915_vm_put(vm);
-                       i915_vma_free(vma);
-                       return pos;
-               }
+               if (!cmp)
+                       goto err_unlock;
 
                if (cmp < 0)
                        p = &rb->rb_right;
@@ -230,8 +226,9 @@ vma_create(struct drm_i915_gem_object *obj,
 err_unlock:
        spin_unlock(&obj->vma.lock);
 err_vma:
+       i915_vm_put(vm);
        i915_vma_free(vma);
-       return ERR_PTR(-E2BIG);
+       return pos;
 }

Andi