[Intel-gfx] [PATCH 3/5] drm/i915: Keep timeline HWSP allocated until idle across the system

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-03-01 11:29:06)
> 
> On 01/03/2019 11:05, Chris Wilson wrote:
> > +int i915_timeline_read_hwsp(struct i915_request *from,
> > +                         struct i915_request *to,
> > +                         u32 *hwsp)
> > +{
> > +     struct i915_timeline_cacheline *cl = from->hwsp_cacheline;
> 
> Is it okay to access the pointer outside the mutex? Below in 
> cacheline_ref it is used.

Hmm, good question. The pointer is safe...
 
> It kind of evaporated what I learnt from the initial review since there 
> is so much new stuff.. :(
> 
> > +     struct i915_timeline *tl = from->timeline;
> > +     int err;
> > +
> > +     GEM_BUG_ON(to->timeline == tl);
> > +
> > +     mutex_lock_nested(&tl->mutex, SINGLE_DEPTH_NESTING);
> > +     err = i915_request_completed(from);

...as we may only release it on retiring the request...

> > +     if (!err)
> > +             err = cacheline_ref(cl, to);
> > +     if (!err) {
> > +             if (likely(cl == tl->hwsp_cacheline)) {
> 
> Or it's here where you check it is still the same?

So to get here, we have proven under the mutex (so no simultaneous
retiring) that the pointer is still valid and current and so safe to
dereference.

It can't be changed on the old request (so no READ_ONCE or mutex
required to store the pointer in a local), but it may be left dangling.
-Chris