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

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Oct 31, 2011 at 01:10:21PM -0400, Ilija Hadzic wrote:
> drm_wait_vblank must be DRM_UNLOCKED because otherwise it
> will grab the drm_global_mutex and then go to sleep until the vblank
> event it is waiting for. That can wreck havoc in the windowing system
> because if one process issues this ioctl, it will block all other
> processes for the duration of all vblanks between the current and the
> one it is waiting for. In some cases it can block the entire windowing
> system.
> 
> v2: incorporate comments received from Daniel Vetter and
>     Michel Daenzer.
> 
> v3: after a lengty discussion with Daniel Vetter, it was concluded
>     we should not worry about any locking, within drm_wait_vblank
>     function so this patch becomes a rather trivial removal
>     of drm_global_mutex from drm_wait_vblank

That commit message is a bit garbage. What about ...

"... it was concluded that the only think not yet protected with locks and
atomic ops is the write to dev->last_vblank_wait. It's only used in a
debug file in proc, and the current code already employs no correct
locking: the proc file only takes dev->struct_mutex, whereas
drm_wait_vblank implicitly took the drm_global_mutex. Given all this, it's
not worth bothering to try to fix this at this time."

I think it's important to correctly document the conclusion of this
discussion, because we've worried quite a bit about correct locking.

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