[Intel-gfx] [PATCH 01/16] drm/i915: Introduce i915_gem_active_wait_unlocked()

[Chris Wilson]

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 <chris at 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