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
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to