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

Ilija Hadzic ihadzic at research.bell-labs.com
Mon Oct 31 14:13:54 PDT 2011


OK, v4 coming up and your suggestion will be copied verbatim. Hopefully 
that does it.

This patch is probably going to become a record-holder in comment/code 
lines ratio ;-)

-- Ilija


On Mon, 31 Oct 2011, Daniel Vetter wrote:

> 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 at ffwll.ch
> Mobile: +41 (0)79 365 57 48
>


More information about the dri-devel mailing list