On to, 2016-04-21 at 08:56 +0100, Chris Wilson wrote:
> On Thu, Apr 21, 2016 at 10:47:30AM +0300, Joonas Lahtinen wrote:
> >
> > On to, 2016-04-21 at 08:08 +0100, Chris Wilson wrote:
> > >
> > > On Thu, Apr 21, 2016 at 09:57:03AM +0300, Joonas Lahtinen wrote:
> > > >
> > > >
> > > > On ke, 2016-04-20 at 19:42 +0100, Chris Wilson wrote:
> > > > >
> > > > >
> > > > > + if (!request->ctx->engine[engine->id].initialised) {
> > > > > + ret = engine->init_context(request);
> > > > > + if (ret) {
> > > > > + intel_lr_context_unpin(request->ctx, engine);
> > > > I prefer the goto teardown path, it's easy to read and modify later on.
> > Meant something like this which is functionally the same but less
> > nesting;
> >
> > if (request->ctx->engine[engine->id].initialised)
> > return 0;
> >
> > ret = engine->init_context(request);
> > if (ret)
> > goto out_unpin;
> >
> > request->ctx->engine[engine->id].initialised = true;
> >
> > return 0;
> >
> > out_unpin:
> > intel_lr_context_unpin(request->ctx, engine);
> >
> > return ret;
> Yes, I am arguing that this leaves the next person thinking it will be
> safe to extend the unwind. And so by not conforming to the idiom, it
> should be more of a danger signal.
>
> Compromise?
Fair enough.
>
> if (!request->ctx->engine[engine->id].initialised) {
> ret = engine->init_context(request);
> if (ret)
> goto err_unpin;
>
> request->ctx->engine[engine->id].initialised = true;
> }
>
> /* Note that after this point, we have committed to using
> * this request as it is being used to both track the
> * state of engine initialisation and liveness of the
> * golden renderstate above. Think twice before you try
> * to cancel/unwind this request now.
> */
>
> return 0;
>
> err_unpin:
> intel_lr_context_unpin(request->ctx, engine);
> return ret;
> }
>
>
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
