[Intel-gfx] [PATCH 35/39] drm/i915: Pin pages before waiting

[Matthew Auld]

On Fri, 14 Jun 2019 at 21:13, Chris Wilson <chris at chris-wilson.co.uk> wrote:
>
> Quoting Chris Wilson (2019-06-14 20:58:09)
> > Quoting Matthew Auld (2019-06-14 20:53:26)
> > > On Fri, 14 Jun 2019 at 08:11, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> > > > @@ -67,10 +61,17 @@ i915_gem_object_set_to_wc_domain(struct drm_i915_gem_object *obj, bool write)
> > > >          * continue to assume that the obj remained out of the CPU cached
> > > >          * domain.
> > > >          */
> > > > -       ret = i915_gem_object_pin_pages(obj);
> > > > +       ret = i915_gem_object_pin_pages_async(obj);
> > > >         if (ret)
> > > >                 return ret;
> > > >
> > > > +       ret = i915_gem_object_wait(obj,
> > > > +                                  I915_WAIT_INTERRUPTIBLE |
> > > > +                                  (write ? I915_WAIT_ALL : 0),
> > > > +                                  MAX_SCHEDULE_TIMEOUT);
> > > > +       if (ret)
> > > > +               goto out_unpin;
> > > > +
> > >
> > > Do we somehow propagate a potential error from a worker to the
> > > object_wait()? Or should we be looking at obj->mm.pages here?
> >
> > Yeah, I've propagated such errors elsewhere (principally along the
> > fences). What you are suggesting is tantamount to making
> > i915_gem_object_wait() report an error, and I have bad memories from all
> > the unhandled -EIO in the past. However, that feels the natural thing to
> > do, so lets give it a whirl.
>
> So we need to check for error pages anyway, because we can't rule out a
> race between the pin_pages_async and i915_gem_object_wait.
>
> There's plenty of duplicated code for pin_pages_async, object_wait,
> check pages so I should refactor that into a variant,
> i915_gem_object_pin_pages_wait() ?

Yeah, makes sense to me.