On Wednesday, January 7, 2009 9:49 am Michel Dänzer wrote:
> On Tue, 2009-01-06 at 22:49 -0800, Keith Packard wrote:
> > On Tue, 2009-01-06 at 10:41 -0800, Jesse Barnes wrote:
> > > This patch adds a sanity check to drmWaitVBlank to prevent hangs.
> > > Since the server interrupts syscalls pretty frequently with SIGALM, we
> > > tend to restart syscalls when EINTR is returned. That means when a
> > > vblank timeout happens, we probably won't catch it (it happens after
> > > 3s, and SIGALM happens every few ms), so we can fairly easily get stuck
> > > restarting the vblank wait ioctl if something fishy is going on (like
> > > vblank interrupts are disabled for some reason).
> > >
> > > So this patch removes the top level restart code, pushing it into the
> > > vblank wait ioctl wrapper, while adding a timeout. If there are cases
> > > where more than 1s waits are desirable, we'd probably need to check the
> > > sequence numbers and come up with a more reasonable value, or add a new
> > > call that takes a timeout parameter.
> >
> > Uh, changing drmIoctl to return on EINTR seems like a bad plan. I'd hack
> > up drmWaitVBlank to do the ioctl directly instead.
>
> Agreed - ignore the comment on this in my previous post, I got that
> backwards of course. :)
Ok; I messed with drmIoctl because I thought I remembered you saying we
shouldn't put the retry code there but rather in the individual ioctls that
needed it. We should probably do that separately anyway though if we ever
decide to.
I think this patch includes the fixes you guys mentioned, I've pushed what you
see below. I thought about adding a warning in the timeout case, but it
would probably be noisy, and the caller can catch the -EBUSY anyway, so I
left it out.
Thanks,
--
Jesse Barnes, Intel Open Source Technology Center
diff --git a/libdrm/Makefile.am b/libdrm/Makefile.am
index eb63abe..71a8718 100644
--- a/libdrm/Makefile.am
+++ b/libdrm/Makefile.am
@@ -23,6 +23,7 @@ SUBDIRS = . intel
libdrm_la_LTLIBRARIES = libdrm.la
libdrm_ladir = $(libdir)
libdrm_la_LDFLAGS = -version-number 2:4:0 -no-undefined
+libdrm_la_LIBADD = -lrt
AM_CFLAGS = -I$(top_srcdir)/shared-core
libdrm_la_SOURCES = xf86drm.c xf86drmHash.c xf86drmRandom.c xf86drmSL.c \
diff --git a/libdrm/xf86drm.c b/libdrm/xf86drm.c
index 0b5d31f..3396e28 100644
--- a/libdrm/xf86drm.c
+++ b/libdrm/xf86drm.c
@@ -42,6 +42,7 @@
#include <fcntl.h>
#include <errno.h>
#include <signal.h>
+#include <time.h>
#include <sys/types.h>
#include <sys/stat.h>
#define stat_t struct stat
@@ -1896,13 +1897,30 @@ int drmScatterGatherFree(int fd, drm_handle_t handle)
*/
int drmWaitVBlank(int fd, drmVBlankPtr vbl)
{
+ struct timespec timeout, cur;
int ret;
+ ret = clock_gettime(CLOCK_MONOTONIC, &timeout);
+ if (ret < 0) {
+ fprintf(stderr, "clock_gettime failed: %s\n", strerror(ret));
+ goto out;
+ }
+ timeout.tv_sec++;
+
do {
- ret = drmIoctl(fd, DRM_IOCTL_WAIT_VBLANK, vbl);
+ ret = ioctl(fd, DRM_IOCTL_WAIT_VBLANK, vbl);
vbl->request.type &= ~DRM_VBLANK_RELATIVE;
+ clock_gettime(CLOCK_MONOTONIC, &cur);
+ /* Timeout after 1s */
+ if (cur.tv_sec > timeout.tv_sec + 1 ||
+ cur.tv_sec == timeout.tv_sec && cur.tv_nsec >= timeout.tv_nsec) {
+ errno = EBUSY;
+ ret = -1;
+ break;
+ }
} while (ret && errno == EINTR);
+out:
return ret;
}
------------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It is the best place to buy or sell services for
just about anything Open Source.
http://p.sf.net/sfu/Xq1LFB
--
_______________________________________________
Dri-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/dri-devel
