On Wed, Aug 03, 2016 at 02:41:05PM +0300, Joonas Lahtinen wrote:
> On ma, 2016-08-01 at 19:22 +0100, Chris Wilson wrote:
> > It is useful to be able to wait on pending rendering without grabbing
> > the struct_mutex. We can do this by using the i915_gem_active_get_rcu()
> > primitive to acquire a reference to the pending request without
> > requiring struct_mutex, just the RCU read lock, and then call
> > i915_wait_request().
> > 
> > Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/i915_gem_request.h | 36 
> > +++++++++++++++++++++++++++++++++
> >  1 file changed, 36 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_gem_request.h 
> > b/drivers/gpu/drm/i915/i915_gem_request.h
> > index 48382ac401fd..d077b023a89f 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_request.h
> > +++ b/drivers/gpu/drm/i915/i915_gem_request.h
> > @@ -546,6 +546,42 @@ i915_gem_active_wait(const struct i915_gem_active 
> > *active, struct mutex *mutex)
> >  }
> >  
> >  /**
> > + * i915_gem_active_wait_unlocked - waits until the request is completed
> > + * @active - the active request on which to wait
> > + * @interruptible - whether the wait can be woken by a userspace signal
> > + * @timeout - how long to wait at most
> > + * @rps - userspace client to charge for a waitboost
> > + *
> > + * i915_gem_active_wait_unlocked() waits until the request is completed 
> > before
> > + * returning, without requiring any locks to be held. Note that it does not
> > + * retire any requests before returning.
> > + *
> > + * This function wraps i915_wait_request(), see it for the full details.
> > + *
> > + * Returns 0 if successful, or a negative error code.
> > + */
> > +static inline int
> > +i915_gem_active_wait_unlocked(const struct i915_gem_active *active,
> > +                         bool interruptible,
> > +                         s64 *timeout,
> > +                         struct intel_rps_client *rps)
> > +{
> > +   struct drm_i915_gem_request *request;
> > +   int ret = 0;
> > +
> > +   rcu_read_lock();
> > +   request = i915_gem_active_get_rcu(active);
> > +   rcu_read_unlock();
> 
> This looks weird compared to the usual way RCU is used, documentation
> explicitly specifies that stuff obtained under rcu_read_lock() can not
> be referenced after rcu_read_unlock(). I'd put the rcu_read_lock()
> section inside i915_gem_active_get_rcu() to make this less confusing.

That's not true for i915_gem_active_get_rcu(). I even did the
rcu_pointer_handoff() just so that it should be clear that the returned
pointer is now protected outside of RCU.

To be clearer then,

static inline struct drm_i915_gem_request *
i915_gem_active_get_rcu(const struct i915_gem_active *active)
{
        struct drm_i915_gem_requst *request;

        rcu_read_lock();
        request = __i915_gem_active_get_rcu(active);
        rcu_read_unlock();

        return request;
}

?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to