On Friday, January 30, 2009 4:47 am Michel Dänzer wrote:
> On Thu, 2009-01-29 at 09:15 -0800, Jesse Barnes wrote:
> > On Thursday, January 29, 2009 12:08 am Michel Dänzer wrote:
> > > On Wed, 2009-01-28 at 20:55 -0800, Jesse Barnes wrote:
> > > > Reported-by: Linus Torvalds <torva...@linux-foundation.org>
> > > > Tested-by: Linus Torvalds <torva...@linux-foundation.org>
> > > > Signed-off-by: Jesse Barnes <jbar...@virtuousgeek.org>
> > >
> > > 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...
>
> I appreciate that, but the above headers suggest there's some history
> we're missing.

Yeah, Linus mailed me off list about a bug that looked at lot like 18041, but 
he wanted his current userspace to work (a reasonable position).

> > > > +       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).
>
> That wouldn't help for the X server hangs I used to get with radeon,
> where the counters were way off due to broken wraparound handling. Are
> you sure the problems you're dealing with aren't that but lost
> interrupts?

I don't think they're wraparound problems in general, no, I think they're 
interrupt enable/disable problems (waiting when interrupts on a given pipe 
are disabled).

I'm not sure about Timo's problem though; this patch helps with a problem he 
was seeing too, but he sees it after running for awhile, not at VT switch.  
It's possible that his problem could be due to wraparound, if the jumps we 
see at interrupt off->on time are big enough, a few DPMS events could cause a 
wraparound related hang.  Timo, do you have a reliable way of reproducing 
your problem?  It sounds separate from 18041 so we may want to fix it 
differently.

> > > 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), [...]
>
> I'm not sure either is necessary. Just store the timeout in the
> file_priv, decrease it when the wait is interrupted and reset it to the
> maximum when the wait finishes. Or could there be several threads
> sharing the same file descriptor?

Yeah, that's one concern.  We'd also have to reset the timeout in the case 
where an interrupted wait ended up getting aborted by the client (i.e. client 
doesn't call back in after -EINTR), followed by a normal wait a bit later.  
That should be detectable though, since a new call would probably have a new 
sequence count, while a restarted call would have the same one.  (Ok, so a 
*small* new mess of code to track this.)

> > 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.
>
> Sure, but this is borderline crippling the core functionality of this
> ioctl.

Yeah, it's borderline, I guess I was hoping it would be on the right side of 
the border enough that I wouldn't have to rewrite it a dozen times. :)

For a time, we just didn't disable vblank interrupts across VT switch (this is 
how we designed the reworked vblank code), but now we do, sigh.

But maybe there's an even simpler way to handle this (the wait on disabled 
pipe/irq problem).  The DRM_WAIT_ON already checks if irqs are disabled 
before waiting (or at least it's supposed to), and the vblank_get call will 
check to make sure the pipe is enabled before waiting... maybe one of those 
checks is failing in this case, or we're not waking clients up at IRQ disable 
time like we should.  I'll do some more testing and see.

-- 
Jesse Barnes, Intel Open Source Technology Center

------------------------------------------------------------------------------
This SF.net email is sponsored by:
SourcForge Community
SourceForge wants to tell your story.
http://p.sf.net/sfu/sf-spreadtheword
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel

Reply via email to