[Intel-gfx] [PATCH 12/16] drm/i915: Reduce locking inside swfinish ioctl

[Chris Wilson]

On Thu, Aug 04, 2016 at 01:32:52PM +0300, Joonas Lahtinen wrote:
> On ma, 2016-08-01 at 19:22 +0100, Chris Wilson wrote:
> >  	/* Pinned buffers may be scanout, so flush the cache */
> > -	if (obj->pin_display)
> > +	if (READ_ONCE(obj->pin_display)) {
> > +		ret = i915_mutex_lock_interruptible(dev);
> > +		if (ret)
> > +			goto unref;
> 
> See below.
> 
> > +
> >  		i915_gem_object_flush_cpu_write_domain(obj);
> >  
> > -	i915_gem_object_put(obj);
> > -unlock:
> > -	mutex_unlock(&dev->struct_mutex);
> > +		i915_gem_object_put(obj);
> > +		mutex_unlock(&dev->struct_mutex);
> > +	} else {
> > +		ret = 0;
> > +unref:
> 
> No, nope, nein, ei, njet, inte, nack; this shall not pass.
> 
> Most inappropriate use of goto I've seen shortly.

It is correct though :-p

Jump forward a few patches, so I can write:

        obj = i915_gem_object_lookup(file, args->handle);
        if (!obj)
                return -ENOENT;

        /* Pinned buffers may be scanout, so flush the cache */
        ret = 0;
        if (READ_ONCE(obj->pin_display)) {
                ret = i915_mutex_lock_interruptible(dev);
                if (ret == 0) {
                        i915_gem_object_flush_cpu_write_domain(obj);
                        mutex_unlock(&dev->struct_mutex);
                }
        }

        i915_gem_object_put(obj);
        return ret;

Hmm, and the struct_mutex there is to protect the obj->base.write_domain.
Maybe cmpxchg...

Anyway, I don't care about swfinish that much, so the above with
put_unlocked for now.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre