Re: [PATCH v2] drm: do not sleep on vblank while holding a mutex

[Daniel Vetter <daniel@ffwll.ch>]

On Sat, Oct 29, 2011 at 06:59:42AM -0500, Ilija Hadzic wrote:
> On Sat, 29 Oct 2011, Daniel Vetter wrote:
> >Ok, and here's why your locking (or any locking that drops the lock before
> >calling schedule) won't work: [SNIP]
> 
> You just came full circle. Recall that in my v1 patch I went all the
> way to enqueuing the process in the wait queue before dropping the
> lock. That would have guaranteed that if there is a hangup, what
> last_vblank_wait says is the last is really the last. If there is no
> hang, then it doesn't matter because last_vlank_wait is constantly
> overwritten (and is indeed stale for N-1 processes). However, that
> was disliked in the review and I didn't want to argue.

Not quite full circle. Holding the lock until you're on the waitqueue
doesn't really change anything - we wake up everybody on every vblank
anyway. The thing I was nitpicking over was that vblank_last_wait isn't
really protected on the read side, but as you've pointed out, that's not a
big problem. Essentially the only thing about vblank_last_wait that we can
guarantee is that somewhen somebody tried to wait on this vblank. Nothing
else is possible (and the reason why I think we should ditch this all and
replace it we some decent tracing - material for a differen patch though).

> So in the interest of making progress, it looks that you would be
> happy if this patch just dropped DRM_UNLOCKED and declared that we
> don't care about (potentially theoretical) critical section related
> to last_vblank_wait.
> 
> If that's the case, I'll cut a relatvely trivial patch that drops
> DRM_UNLOCKED from this system call to solve the problem that I
> pointed earlier in this thread and leave all the rest of the locking
> discussion for other patches.
> 
> Would that be fine by you ?

Yeah.

Cheers, Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48