[Intel-gfx] [PATCH 11/16] drm/i915: Remove (struct_mutex) locking for busy-ioctl

[Chris Wilson]

On Thu, Aug 04, 2016 at 01:25:08PM +0300, Joonas Lahtinen wrote:
> On ma, 2016-08-01 at 19:22 +0100, Chris Wilson wrote:
> > +static __always_inline unsigned
> > +__busy_read_flag(const struct drm_i915_gem_request *request)
> > +{
> > +	return 0x10000 << request->engine->exec_id;
> 
> Duh, this really is our ABI? No BIT(NUM_ENGINES) or something sane?

BIT(NUM_ENGINES) as uabi, you call that sane :)

> Make a comment of such situation.

Like BUILD_BUG_ON(NUM_ENGINES > 16).

> > +		/* Yes, the lookups are intentionally racy.
> > +		 *
> > +		 * Even though we guard the pointer lookup by RCU, that only
> > +		 * guarantees that the pointer and its contents remain
> > +		 * dereferencable and does *not* mean that the request we
> > +		 * have is the same as the one being tracked by the object.
> > +		 *
> > +		 * Consider that we lookup the request just as it is being
> > +		 * retired and free. We take a local copy of the pointer,
> 
> s/free/freed/
> 
> > +		 * but before we add its engine into the busy set, the other
> > +		 * thread reallocates it and assigns it to a task on another
> > +		 * engine with a fresh and incomplete seqno.
> > +		 *
> > +		 * So after we lookup the engine's id, we double check that
> 
> This double check is nowhere to be seen, time to update this comment
> too?

Actualy snuck it back in. I took it out thinking that there wasn't
sufficent read-dependency to guarrantee the ordering (but now reversed
my decision there) and had re-read my comment about why the check is
required.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre