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
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
