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

[Daniel Vetter]

On Fri, Oct 28, 2011 at 09:44:54PM -0500, Ilija Hadzic wrote:
> Now suppose that the drm_wait_vblank is enteret in the context of
> two different PIDs and suppose that there are no locks. Let's say
> that the first process wants to wait on vblank N and the second
> wants to wait on vblank N+1. First process can reach the point just
> before it wants to write to last_vblank_wait and lose the processor
> there (so it has N in vblank.request (on its stack) calculated, but
> it has not written it into last_vblank_wait yet (because it lost the
> CPU right there). Then the second process gets the CPU, and executes
> and let's say that it goes all the way through so it writes N+1 into
> last_vblank_wait and then schedules away (it called DRM_WAIT_ON).
> Now the first process gets the CPU where it left off and overwrites
> N+1 in last_vblank_wait with N, which is now stale.

Ok, and here's why your locking (or any locking that drops the lock before
calling schedule) won't work: Dropping the lock allows more than one
process waiting for a vblank on the same crtc (the hole point of this
exerices). So let's say N processes are waiting for vblank on crtc A, but
we only have one storage place for last_vblank_wait. So by necessity, it's
gonna be stale for N-1 processes. And because the scheduler is free to
take away the cpu between the unlock and the schedule, you can freely pick
for which process it's gonna be the right one, there's zero ordering
guarantees possible.

By throwing in a modeset call that disables vblanks it's pretty easy to
come up with a process that waits for vblank x+1 (when x is the last
vblank before the vblank counter got stopped before the modeset) and
last_vblank_wait == x.

Which is why I think adding decent tracepoints would be much better, but
that's a different patch.

[snip]

Agreed on not locking the debug file and postponing other cleanups, that's
all material for different patches.

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