Quoting Mika Kuoppala (2019-05-22 15:32:34)
> Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > index 5016a3e1f863..a9608d9ced6a 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
> > @@ -95,24 +95,42 @@ void i915_lut_handle_free(struct i915_lut_handle *lut)
> >
> > static void lut_close(struct i915_gem_context *ctx)
> > {
> > - struct i915_lut_handle *lut, *ln;
> > struct radix_tree_iter iter;
> > void __rcu **slot;
> >
> > - list_for_each_entry_safe(lut, ln, &ctx->handles_list, ctx_link) {
> > - list_del(&lut->obj_link);
> > - i915_lut_handle_free(lut);
> > - }
> > - INIT_LIST_HEAD(&ctx->handles_list);
> > + lockdep_assert_held(&ctx->mutex);
> >
> > rcu_read_lock();
> > radix_tree_for_each_slot(slot, &ctx->handles_vma, &iter, 0) {
> > struct i915_vma *vma = rcu_dereference_raw(*slot);
> > -
> > - radix_tree_iter_delete(&ctx->handles_vma, &iter, slot);
> > -
> > - vma->open_count--;
> > - i915_vma_put(vma);
> > + struct drm_i915_gem_object *obj = vma->obj;
> > + struct i915_lut_handle *lut;
> > + bool found = false;
> > +
> > + rcu_read_unlock();
> > + i915_gem_object_lock(obj);
> > + list_for_each_entry(lut, &obj->lut_list, obj_link) {
> > + if (lut->ctx != ctx)
> > + continue;
> > +
> > + if (lut->handle != iter.index)
> > + continue;
> > +
> > + list_del(&lut->obj_link);
> > + i915_lut_handle_free(lut);
>
> You could free this after object_unlock if you want. Shrug.
>
> > + found = true;
> > + break;
> > + }
> > + i915_gem_object_unlock(obj);
> > + rcu_read_lock();
> > +
> > + if (found) {
> > + radix_tree_iter_delete(&ctx->handles_vma, &iter, slot);
> > + if (atomic_dec_and_test(&vma->open_count) &&
> > + !i915_vma_is_ggtt(vma))
> > + i915_vma_close(vma);
> > + i915_gem_object_put(obj);
>
> I am strugging to pair this with get.
[snip]
> > #endif /* __I915_GEM_CONTEXT_TYPES_H__ */
> > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > index 13ab2a2e0099..fa0a880ca4de 100644
> > --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
> > @@ -801,9 +801,6 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb)
> > unsigned int i, batch;
> > int err;
> >
> > - if (unlikely(i915_gem_context_is_closed(eb->gem_context)))
> > - return -ENOENT;
> > -
> > if (unlikely(i915_gem_context_is_banned(eb->gem_context)))
> > return -EIO;
> >
> > @@ -812,6 +809,12 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb)
> >
> > batch = eb_batch_index(eb);
> >
> > + mutex_lock(&eb->gem_context->mutex);
> > + if (unlikely(i915_gem_context_is_closed(eb->gem_context))) {
> > + err = -ENOENT;
> > + goto err_ctx;
> > + }
> > +
> > for (i = 0; i < eb->buffer_count; i++) {
> > u32 handle = eb->exec[i].handle;
> > struct i915_lut_handle *lut;
> > @@ -846,12 +849,14 @@ static int eb_lookup_vmas(struct i915_execbuffer *eb)
> > }
> >
> > /* transfer ref to ctx */
It is this reference that we transfer from the object lookup into the
lut, that we must release if we throwaway the lut.
> > - if (!vma->open_count++)
> > + if (!atomic_fetch_inc(&vma->open_count))
> > i915_vma_reopen(vma);
> > - list_add(&lut->obj_link, &obj->lut_list);
> > - list_add(&lut->ctx_link, &eb->gem_context->handles_list);
> > - lut->ctx = eb->gem_context;
> > lut->handle = handle;
> > + lut->ctx = eb->gem_context;
> > +
> > + i915_gem_object_lock(obj);
> > + list_add(&lut->obj_link, &obj->lut_list);
> > + i915_gem_object_unlock(obj);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
