[Intel-gfx] [PATCH 63/70] drm/i915: Reduce locking inside swfinish ioctl

[Chris Wilson]

On Wed, Apr 15, 2015 at 11:33:33AM +0200, Daniel Vetter wrote:
> On Wed, Apr 15, 2015 at 10:03:56AM +0100, Chris Wilson wrote:
> > On Fri, Apr 10, 2015 at 11:14:56AM +0200, Daniel Vetter wrote:
> > > On Tue, Apr 7, 2015 at 6:28 PM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> > > >         /* Pinned buffers may be scanout, so flush the cache */
> > > > -       if (obj->pin_display)
> > > > +       if (obj->pin_display) {
> > > > +               ret = i915_mutex_lock_interruptible(dev);
> > > > +               if (ret)
> > > > +                       goto unref;
> > > 
> > > I think and ACCESS_ONCE here and in the previous one would be good.
> > > Wanted to do that and merge both, but they seem to conflict with the
> > > lack of read-read ...
> > 
> > What do you want to accomplish with ACCESS_ONCE()? Unfortunately as we
> > can't use it on bitfields.
> 
> making sure that gcc doesn't reload the variable when it shuffles basic
> blocks around. Admittedly unlikely. The point is not to do an atomic load
> but just to ensure that we have a consistent value, no matter how obtained
> so. But I guess we need to open-code the ACCESS_ONCE with a (volatile
> bool) cast.
> 
> The other reason is that it serves as a nice reminder to the reader that
> something tricky is going on, and that the lockless reading of
> ->pin_display is intentional.

Ok, that's just what I thought you wanted. We can hope that the
optimistic_read() is more friendly (presupposing Linus actually adds
it).
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre