Re: [PATCH 3/3] drm/i915: Ignore duplicate VMA stored within the per-object handle LUT

Chris Wilson
2017-08-30T11:56:55Z

Quoting Joonas Lahtinen (2017-08-30 12:07:47)
> On Wed, 2017-08-23 at 11:20 +0100, Chris Wilson wrote:
> > Quoting Joonas Lahtinen (2017-08-23 11:05:18)
> > > On Tue, 2017-08-22 at 12:05 +0100, Chris Wilson wrote:
> > > > By using drm_gem_flink/drm_gem_open on an object using the same fd, it
> > > > is possible for a client to create multiple handles pointing to the same
> > > > object (tied to the same contexts and VMA), as exemplified by
> > > > igt::gem_handle_to_libdrm_bo(). Since this duplication has been possible
> > > > since forever, we cannot assume that the handle:(fpriv, object) is
> > > > unique and so must handle the multiple users of a single VMA.
> > > >
> > > > Testcase: igt/gem_close
> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102355
> > > > Fixes: d1b48c1e7184 ("drm/i915: Replace execbuf vma ht with an idr")
> > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > > > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > >
> > > <SNIP>
> > >
> > > > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > > > @@ -720,6 +720,7 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb)
> > > > goto err_obj;
> > > > }
> > > >
> > > > + vma->open_count++;
> > > > list_add(&lut->obj_link, &obj->lut_list);
> > >
> > > This code maybe should be in i915_gem.c as "i915_gem_object_add_lut" or
> > > something.
> >
> > I disagree. It's very much tied to being an execbuf only interaction,
> > that obj/ctx/handle.
>
> So how are we going to proceed here? The current proposed solution is
> very unintuitive, one counter spread over multiple files.

The table is very much for the entertainment of execbuf (and if you
squint hard, ok not hard at all, so is the rest of GEM), if you were to
push hard that's where I suggest to shove it.

But I'm not yet seeing the issue with one side being clear where the
user opens the vma and the other where it is closed by the user.
-Chris