On Thu, Sep 5, 2013 at 6:24 PM, Daniel Vetter <daniel@xxxxxxxx> wrote: > On Thu, Sep 5, 2013 at 4:50 PM, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote: >>> > + /* Do an optimistic check for activity - we don't care about userspace >>> > + * racing with itself, that is always problematic. >>> > + */ >>> > + ring = obj->ring; >>> > + if (ring && obj->last_read_seqno == ring->outstanding_lazy_seqno) >>> > + goto lock_and_flush; >>> >>> Feels a bit too tricky ... How useful is just an >>> >>> if (ACCESS_ONCE(obj->active)) >>> goto lock_and_flush; >> >> Not good enough, still ends up fighting for the lock. > > Pondering this some more the problem is that I don't think we can > ignore racing userspace since it can legitimately race here, e.g. > - dri2 client uses the busy check to figure out whether it can access > the frontbuffer for pixel readback > - X server does a pageflip/blt which results in a ring switch > > If now the ring we switch to is ahead of the old ring with signalling > seqno completion and the lockless busy reads the ring and seqno out > of order we'll report the buffer falsely as non-busy. And the above > scenario doesn't involve anything userspace shouldn't do. > > So I think reading just obj->ring would be ok, but doing more outside > of proper locking won't work out. For that we need to bite the bullet > and install real per-object locking, or something else suitable. Also > I'm reluctant to merge clever code that would be superseeded by a > locking rework. The shrinker madness is an exception since we simply > need those quick fixes for correctness. Checking for obj->ring outside > of any locks would imo still be useful as a micro-optimization when we > have per-object locking. > > The lockless waiting in set_domain is already a bit on the fence for > me, but this here feels like too much trickery just to paper over the > performance issues of our too simplistic locking scheme ... Another option would be to switch to using requests instead of ring/seqno pairs. Since those are essentially invariant we could just protect them from disappearing with rcu and kref_get_unless_zero trickery ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
