From: Jesse B. <jb...@vi...> - 2009-01-29 17:16:12
|
On Thursday, January 29, 2009 12:08 am Michel Dänzer wrote: > On Wed, 2009-01-28 at 20:55 -0800, Jesse Barnes wrote: > > Per fdo bz #18041, we timeout if a vblank event doesn't occur within a > > specified time period. However, this wait happens in libdrm, so old > > userspace can still end up hanging, waiting for a vblank event that may > > never occur. So add a more reasonable timeout value to drm_wait_vblank > > and make the wait uninterruptible, so that the X server's scheduling > > signals don't get in the way. > > I'm afraid I don't like it. > > > Reported-by: Linus Torvalds <tor...@li...> > > Tested-by: Linus Torvalds <tor...@li...> > > Signed-off-by: Jesse Barnes <jb...@vi...> > > It would be nice if things like this could be discussed on this list in > the first place... Well that's why I posted it here... > > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c > > index 69aa0ab..c5df063 100644 > > --- a/drivers/gpu/drm/drm_irq.c > > +++ b/drivers/gpu/drm/drm_irq.c > > @@ -539,9 +539,11 @@ out: > > int drm_wait_vblank(struct drm_device *dev, void *data, > > struct drm_file *file_priv) > > { > > + struct timeval now; > > union drm_wait_vblank *vblwait = data; > > int ret = 0; > > unsigned int flags, seq, crtc; > > + int timeout, max_wait = (DRM_HZ / 40) * 100; /* 100ms max */ > > Is that really a more reasonable timeout? It means e.g. the > GLX_SGI_swap_control extension can't be used for swap intervals longer > than 100 ms. Now I wouldn't expect such long intervals to be needed > often, but it doesn't seem completely useless either (e.g. it allows > apps which only need to update their display a couple of times per > second to have a trivial rendering loop). > > Also, blocking signals for up to 100 ms sounds like bad news for the X > server scheduler and SIGIO driven mouse cursor updates. If the server does long waits, yeah it could affect scheduling. But really there are two problems here: one is the signal problem that causes an infinite loop of never completed vblank waits, and another is lots missed interrupts causing apps to hang for awhile. Maybe we could do an interruptible wait if the framecount was greater than say 10, but do an uninterruptible one with a small max timeout for less (numbers are arbitrarily picked here). > Maybe the kernel code could remember how long it waited before being > interrupted by a signal and then decrease that from the timeout next > time or something like that? Probably not without a new arg to the ioctl like Thomas said (or a huge new mess of code to track processes what what we think they want), which would be hard to do in a backward compatible way. And keep in mind that vblank waits like this are inherently more racy than a good swapbuffers implementation, since they depend on the app getting scheduled in time to actually avoid tearing. -- Jesse Barnes, Intel Open Source Technology Center |