Quoting Tvrtko Ursulin (2019-09-26 14:57:24)
>
> On 25/09/2019 11:01, Chris Wilson wrote:
> > void i915_gem_context_release(struct kref *ref)
> > {
> > struct i915_gem_context *ctx = container_of(ref, typeof(*ctx), ref);
> > - struct drm_i915_private *i915 = ctx->i915;
> > + struct i915_gem_contexts *gc = &ctx->i915->gem.contexts;
> >
> > trace_i915_context_free(ctx);
> > - if (llist_add(&ctx->free_link, &i915->contexts.free_list))
> > - queue_work(i915->wq, &i915->contexts.free_work);
> > + if (llist_add(&ctx->free_link, &gc->free_list))
> > + queue_work(ctx->i915->wq, &gc->free_work);
>
> At first I thought gc was some sort of garbage collection list. But it
> is a GEM contexts list. :) This hunk looks completely avoidable, I don't
> see it brings much benefit on balance since in turn it adds ctx->i915
> twice . But as you wish..
Since it's not taking struct_mutex it doesn't need i915->wq any more,
good point.
> > @@ -464,11 +454,11 @@ static void __apply_ppgtt(struct intel_context *ce, void *vm)
> > static struct i915_address_space *
> > __set_ppgtt(struct i915_gem_context *ctx, struct i915_address_space *vm)
> > {
> > - struct i915_address_space *old = ctx->vm;
> > + struct i915_address_space *old = rcu_dereference_protected(ctx->vm, 1);
> >
> > GEM_BUG_ON(old && i915_vm_is_4lvl(vm) != i915_vm_is_4lvl(old));
> >
> > - ctx->vm = i915_vm_open(vm);
> > + rcu_assign_pointer(ctx->vm, i915_vm_open(vm));
>
> Are updaters here serialized? Lost track if struct mutex is still held
> in set_ppgtt at this point in the series or not.. Thinking of "old =
> rcu_dereference_protected.." and here.
Yes, ctx->mutex; I was being very lazy in not marking it up; especially
in the selftests where it was so much more dontcare.
> > @@ -653,8 +633,8 @@ static int gem_context_register(struct i915_gem_context *ctx,
> > int ret;
> >
> > ctx->file_priv = fpriv;
> > - if (ctx->vm)
> > - ctx->vm->file = fpriv;
> > + if (rcu_access_pointer(ctx->vm))
> > + rcu_dereference_protected(ctx->vm, true)->file = fpriv;
>
> Here rcu accesses are just to satisfy sparse?
Total eyesore, yup. I'm thinking I just declare ctx->vm to be protected
by ctx->mutex and make it so.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
