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 >
