On Die, 2011-10-25 at 22:20 -0400, Ilija Hadzic wrote:
> holding the drm_global_mutex in drm_wait_vblank and then
> going to sleep (via DRM_WAIT_ON) is a bad idea in general
> because it can block other processes that may issue ioctls
> that also grab drm_global_mutex. Blocking can occur until
> next vblank which is in the tens of microseconds order of
> magnitude.
>
> fix it, but making drm_wait_vblank DRM_UNLOCKED call and
> then grabbing the mutex inside the drm_wait_vblank function
> and only for the duration of the critical section (i.e.,
> from first manipulation of vblank sequence number until
> putting the task in the wait queue).
Does it really need drm_global_mutex at all, as opposed to e.g.
dev->struct_mutex?
> diff --git a/include/drm/drm_os_linux.h b/include/drm/drm_os_linux.h
> index 3933691..fc6aaf4 100644
> --- a/include/drm/drm_os_linux.h
> +++ b/include/drm/drm_os_linux.h
> @@ -123,5 +123,30 @@ do
> { \
> remove_wait_queue(&(queue), &entry); \
> } while (0)
>
> +#define DRM_WAIT_ON_LOCKED( ret, queue, timeout, condition ) \
> +do { \
> + DECLARE_WAITQUEUE(entry, current); \
> + unsigned long end = jiffies + (timeout); \
> + add_wait_queue(&(queue), &entry); \
> + mutex_unlock(&drm_global_mutex); \
I agree with Daniel's sentiment on this. AFAICT add_wait_queue()
protects against concurrent access to the wait queue, so I think it
would be better to just drop the mutex explicitly before calling
DRM_WAIT_ON.
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Debian, X and DRI developer
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
