On Fri, 2022-07-01 at 15:50 -0700, Niranjana Vishwanathapura wrote:
> vma_lookup is tied to segment of the object instead of section
> of VA space. Hence, it do not support aliasing (ie., multiple
> bindings to the same section of the object).
> Skip vma_lookup for persistent vmas as it supports aliasing.
>
> Signed-off-by: Niranjana Vishwanathapura
> <niranjana.vishwanathapura@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_vma.c | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_vma.c
> b/drivers/gpu/drm/i915/i915_vma.c
> index 6adb013579be..9aa38b772b5b 100644
> --- a/drivers/gpu/drm/i915/i915_vma.c
> +++ b/drivers/gpu/drm/i915/i915_vma.c
> @@ -197,6 +197,10 @@ vma_create(struct drm_i915_gem_object *obj,
> __set_bit(I915_VMA_GGTT_BIT, __i915_vma_flags(vma));
> }
>
> + if (!i915_vma_is_ggtt(vma) &&
> + (view && view->type == I915_GGTT_VIEW_PARTIAL))
> + goto skip_rb_insert;
> +
Rather than guessing that a vma with this signature is a persistent
vma, which is confusing to the reader, could we have an argument saying
we want to create a persistent vma?
> rb = NULL;
> p = &obj->vma.tree.rb_node;
> while (*p) {
> @@ -221,6 +225,7 @@ vma_create(struct drm_i915_gem_object *obj,
> rb_link_node(&vma->obj_node, rb, p);
> rb_insert_color(&vma->obj_node, &obj->vma.tree);
>
> +skip_rb_insert:
> if (i915_vma_is_ggtt(vma))
> /*
> * We put the GGTT vma at the start of the vma-list,
> followed
> @@ -292,13 +297,16 @@ i915_vma_instance(struct drm_i915_gem_object
> *obj,
> struct i915_address_space *vm,
> const struct i915_ggtt_view *view)
> {
> - struct i915_vma *vma;
> + struct i915_vma *vma = NULL;
>
> GEM_BUG_ON(!kref_read(&vm->ref));
>
> - spin_lock(&obj->vma.lock);
> - vma = i915_vma_lookup(obj, vm, view);
> - spin_unlock(&obj->vma.lock);
> + if (i915_is_ggtt(vm) || !view ||
> + view->type != I915_GGTT_VIEW_PARTIAL) {
Same here?
/Thomas
> + spin_lock(&obj->vma.lock);
> + vma = i915_vma_lookup(obj, vm, view);
> + spin_unlock(&obj->vma.lock);
> + }
>
> /* vma_create() will resolve the race if another creates the
> vma */
> if (unlikely(!vma))
> @@ -1670,7 +1678,8 @@ static void release_references(struct i915_vma
> *vma, bool vm_ddestroy)
>
> spin_lock(&obj->vma.lock);
> list_del(&vma->obj_link);
> - if (!RB_EMPTY_NODE(&vma->obj_node))
> + if (!i915_vma_is_persistent(vma) &&
> + !RB_EMPTY_NODE(&vma->obj_node))
> rb_erase(&vma->obj_node, &obj->vma.tree);
>
> spin_unlock(&obj->vma.lock);
