On Thu, 15 Dec 2016, Thierry Reding <thierry.reding@xxxxxxxxx> wrote:
> From: Thierry Reding <treding@xxxxxxxxxx>
>
> Ever since support for vblank handling and event processing was added to
> libdrm (v2.4.16), events were read in 1 KiB chunks in drmHandleEvent().
> In addition, the implementation of drm_read() will not unqueue events if
> they can't be successfully passed to userspace. The result is that it's
> possible for a driver to send applications into a busy loop by creating
> events larger than 1 KiB.
>
> There is a limit to the event space that each DRM device can carry, but
> it is set to 4 KiB.
>
> While no driver currently produces events larger than 1 KiB, and it's
> fairly unlikely that anyone ever will, it's best to err on the side of
> caution and limit the size of individual events to 1 KiB. This ensures
> that libdrm will never run into the busy loop. In order to allow for
> the DRM device to queue multiple events, the event space limit is kept
> at 4 KiB.
>
> A complementary patch will be applied to libdrm to bump the read size
> for events to 4 KiB to ensure that newer versions of libdrm running on
> kernels without this fix will not run into the busy loop either.
>
> Signed-off-by: Thierry Reding <treding@xxxxxxxxxx>
> ---
> drivers/gpu/drm/drm_fops.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_fops.c b/drivers/gpu/drm/drm_fops.c
> index 48e106557c92..3792e4434319 100644
> --- a/drivers/gpu/drm/drm_fops.c
> +++ b/drivers/gpu/drm/drm_fops.c
> @@ -582,6 +582,15 @@ EXPORT_SYMBOL(drm_poll);
> * This is the locked version of drm_event_reserve_init() for callers which
> * already hold dev->event_lock.
> *
> + * Note that libdrm has been reading events in chunks of 1 KiB since forever
> + * (see drmHandleEvent()) and up to and including v2.4.74. Unfortunately the
Maybe it's a case of post-lunch catatonia, but I actually read that a
few times, and thought it made no sense, until I realized the version
number refers to libdrm, not kernel... Would it be too verbose to use
"libdrm v2.4.74" explicitly here and below?
BR,
Jani.
> + * drm_read() implementation won't return an error if an event doesn't fit
> + * into that 1 KiB and instead these events will simply be queued again. If
> + * a driver was to send out an event larger than 1 KiB, an application that
> + * polls the DRM file descriptor and calling drmHandleEvent() to process the
> + * events would go into a busy loop. This is caused by drm_poll() always
> + * return immediately, but drm_read() never being able to dequeue the event.
> + *
> * RETURNS:
> *
> * 0 on success or a negative error code on failure.
> @@ -591,6 +600,15 @@ int drm_event_reserve_init_locked(struct drm_device *dev,
> struct drm_pending_event *p,
> struct drm_event *e)
> {
> + /*
> + * Limit events to 1 KiB, which is the read size of libdrm (up to
> + * to and including v2.4.74) when processing events, in order to
> + * prevent any of the applications using libdrm from potentially
> + * going into a busy loop.
> + */
> + if (e->length > 1024)
> + return -EINVAL;
> +
> if (file_priv->event_space < e->length)
> return -ENOMEM;
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
