Chris Wilson <ch...@chris-wilson.co.uk> writes:
> Quoting Mika Kuoppala (2018-08-16 12:42:08)
>> Chris Wilson <ch...@chris-wilson.co.uk> writes:
>>
>> > The context owns both the ppgtt and the vma within it, and our activity
>> > tracking on the context ensures that we do not release active ppgtt. As
>> > the context fulfils our obligations for active memory tracking, we can
>> > relinquish the reference from the vma.
>> >
>>
>> The part of owning the vma within it escapes me. The vma is tied
>> to the object. Is it about that active objects, with their vmas
>> hold the refs to the context they are executing on?
>
> The vma belongs to the (object, context). If the context is closed (or
> on close(/dev/dri/card0)), so is the ppgtt. Closing the object
> (gem_close(fd, obj)) closes the vma that match the contexts for the fd,
> and if it was the last handle, then frees the object.
>
>> > This fixes a silly transient refleak from closed vma being kept alive
>> > until the entire system was idle, keeping all vm alive as well.
>> >
>> > Reported-by: Paulo Zanoni <paulo.r.zan...@intel.com>
>> > Testcase: igt/gem_ctx_create/files
>> > Fixes: 3365e2268b6b ("drm/i915: Lazily unbind vma on close")
>> > Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
>> > Cc: Tvrtko Ursulin <tvrtko.ursu...@intel.com>
>> > Cc: Paulo Zanoni <paulo.r.zan...@intel.com>
>> > ---
>> > drivers/gpu/drm/i915/i915_vma.c | 4 ----
>> > 1 file changed, 4 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_vma.c
>> > b/drivers/gpu/drm/i915/i915_vma.c
>> > index 274fd2a7bcb6..31efc971a3a8 100644
>> > --- a/drivers/gpu/drm/i915/i915_vma.c
>> > +++ b/drivers/gpu/drm/i915/i915_vma.c
>> > @@ -199,7 +199,6 @@ vma_create(struct drm_i915_gem_object *obj,
>> > vma->flags |= I915_VMA_GGTT;
>> > list_add(&vma->obj_link, &obj->vma_list);
>> > } else {
>> > - i915_ppgtt_get(i915_vm_to_ppgtt(vm));
>>
>> It seems this is the sole user of i915_ppgtt_get so you can
>> remove that too.
>
> Such shortsightedness; it will reappear shortly. See the patches to
> share vm between contexts.
Ok let it float in there for a while.
Reviewed-by: Mika Kuoppala <mika.kuopp...@linux.intel.com>
Also
Tested-by: Mika Kuoppala <mika.kuopp...@linux.intel.com>
I also been after this one previously and managed to bring
up a testcase where we create a mapping to an object and
then create context, do execbuf with mapped object as relocatable,
and destroy context. That would also end up keeping ppgtts
alive until client closes.
Would it be worthwhile to include such test or do you think
gem_ctx_create@files covers enough?
-Mika
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
