Quoting Andi Shyti (2020-07-02 21:25:45)
> 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;
Yeah, but you might as well do
if (cmp < 0)
p = right;
else if (cmp > 0)
p = left;
else
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
Ta, going to send that as a patch?
-Chris
