Quoting Tvrtko Ursulin (2019-03-18 16:22:12)
>
> On 18/03/2019 09:51, Chris Wilson wrote:
> > +static int gem_context_register(struct i915_gem_context *ctx,
> > + struct drm_i915_file_private *fpriv)
> > +{
> > + int ret;
> > +
>
> Assert struct mutex for now? Without it userspace can still see not
> fully initialized ctx. It is kind of two arguments but good for
> documentation nevertheless I think.
The goal is that we need to fix that now. And we can't hold struct_mutex
across the extensions, as we want to wrap ctx_setparam which expects to
be able to take struct_mutex. So it has to be registered outside of
struct_mutex in this or the next path.
> > + ctx->pid = get_task_pid(current, PIDTYPE_PID);
> > +
> > + if (ctx->ppgtt)
> > + ctx->ppgtt->vm.file = fpriv;
>
> Thinking about i915_vm_set_file(vm, fpriv), not sure.
Nah, longer term needs a better fix since ppgtt is shared. (I'm just
ignoring this for now, since they all must have the same fpriv.)
> > +
> > + /* And (nearly) finally expose ourselves to userspace via the idr */
> > + ret = idr_alloc(&fpriv->context_idr, ctx,
> > + DEFAULT_CONTEXT_HANDLE, 0, GFP_KERNEL);
> > + if (ret < 0)
> > + goto err_pid;
> > +
> > + ctx->file_priv = fpriv;
> > + ctx->user_handle = ret;
> > +
> > + ctx->name = kasprintf(GFP_KERNEL, "%s[%d]/%x",
> > + current->comm,
> > + pid_nr(ctx->pid),
> > + ctx->user_handle);
> > + if (!ctx->name) {
> > + ret = -ENOMEM;
> > + goto err_idr;
> > + }
> > +
> > + return 0;
> > +
> > +err_idr:
> > + idr_remove(&fpriv->context_idr, ctx->user_handle);
> > + ctx->file_priv = NULL;
> > +err_pid:
> > + put_pid(ctx->pid);
> > + ctx->pid = NULL;
>
> To avoid this, and in the spirit of the patch, I think it would be
> better to just store everything in locals and assign to ctx members once
> passed the point of failure.
Ah. Yes. That should solve the problem of that idr_alloc() not being
last as it needs to be.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
