[Intel-gfx] [PATCH 3/3] drm/i915: Replace execbuf vma ht with an idr

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-06-20 12:26:44)
> 
> On 16/06/2017 17:02, Chris Wilson wrote:
> > -static void resize_vma_ht(struct work_struct *work)
> > +static void lut_close(struct i915_gem_context *ctx)
> >   {
> > -     struct i915_gem_context_vma_lut *lut =
> > -             container_of(work, typeof(*lut), resize);
> > -     unsigned int bits, new_bits, size, i;
> > -     struct hlist_head *new_ht;
> > -
> > -     GEM_BUG_ON(!(lut->ht_size & I915_CTX_RESIZE_IN_PROGRESS));
> > -
> > -     bits = 1 + ilog2(4*lut->ht_count/3 + 1);
> > -     new_bits = min_t(unsigned int,
> > -                      max(bits, VMA_HT_BITS),
> > -                      sizeof(unsigned int) * BITS_PER_BYTE - 1);
> > -     if (new_bits == lut->ht_bits)
> > -             goto out;
> > -
> > -     new_ht = kzalloc(sizeof(*new_ht)<<new_bits, GFP_KERNEL | __GFP_NOWARN);
> > -     if (!new_ht)
> > -             new_ht = vzalloc(sizeof(*new_ht)<<new_bits);
> > -     if (!new_ht)
> > -             /* Pretend resize succeeded and stop calling us for a bit! */
> > -             goto out;
> > +     struct i915_lut_handle *lut, *ln;
> > +     struct radix_tree_iter iter;
> > +     void __rcu **slot;
> >   
> > -     size = BIT(lut->ht_bits);
> > -     for (i = 0; i < size; i++) {
> > -             struct i915_vma *vma;
> > -             struct hlist_node *tmp;
> > -
> > -             hlist_for_each_entry_safe(vma, tmp, &lut->ht[i], ctx_node)
> > -                     hlist_add_head(&vma->ctx_node,
> > -                                    &new_ht[hash_32(vma->ctx_handle,
> > -                                                    new_bits)]);
> > +     list_for_each_entry_safe(lut, ln, &ctx->handles_list, ctx_link) {
> > +             list_del(&lut->obj_link);
> I assume you deliberately did not bother unlinking from the ctx list 
> since all that is getting zapped anyway?

Yes, it is not obvious though. The context is marked as closed so we
should thereafter prevent further usage of the ctx->handles_list.

At the moment, this is governed by struct_mutex but in the future it is
likely to be replaced by file->mutex or ctx->mutex.

If I add a GEM_BUG_ON(eb->ctx->closed) to eb_lookup_vmas that should
help explain the relationship.
-Chris