On Monday, February 2, 2009 2:49 pm Michel Dänzer wrote: > Actually, I'm afraid I was getting confused... Looking at the current > code again, it does about the opposite of what I said above: it > terminates the wait if the target sequence is up to a day behind the > counter, so it would allow waiting for up to more than two years. > > However, if we are to change this test, I'd suggest going much further > than your proposal (which btw is really just a fancy way of replacing > the shift of 23 with 31 :): e.g. something like > > (vblwait->request.sequence - drm_vblank_count(dev, crtc) - 1) > (1 << 23) > > should do what I said above. Then if we reduce the right hand side to > e.g. 3600, only waiting for up to 3600 frames (one minute at 60 Hz) is > possible, anything else is considered a miss. > > (I hope I didn't make any stupid mistake in the math again, it's getting > late... but I'm quite confident the principle is sound)
Ok let's go through a few cases with your above test (the 8 million count one), I think you got the math right with the corrected test above :): a) request greater than current sequence (the typical one hopefully) b) request less than current sequence c) request greater than current sequence, but due to wrapping (vblwait->request.sequence - drm_vblank_count(dev, crtc) - 1) > (1 << 23) So for case (a) with say request = 500 vs cur = 482, we'd have (500 - 482 - 1) > (1 << 23), which is false, so we'd stay in the loop, good. And for case (b), say request = 320 and cur = 698, we'd have (320 - 698 - 1) > (1 << 23), which is true, so we'd break out, good (this even works for large cur vs. request differences). And for case (c), say request = 8388611 and cur = 1, we'd have (8388611 - 1 - 1) > (1 << 23), which is true, so we'd break out. Again good. And yes, the patch I posted is a "fancy" but also typical way of doing this (just look around the kernel, it's a pretty common idiom to cast stuff to signed and compare against 0). As for the higher level question, I hope we both agree that we should fix this up, though I'm not sure why we need a frame count cap at all. We've already got the timeout, so shouldn't we just keep the code simple? Is debugging tearing easier than debugging a hang? I guess that's what we're really getting at... > > The jumping counter exposed that problem and should probably be fixed > > too (just to keep the counter values more friendly) but the core > > problem should be fixed first, imo. > > I still disagree there: If the counter doesn't jump, even the current > code works just fine as long as userspace calls the ioctl at least once > a day. We can try to contain the damage caused by a jumping counter, but > we can't completely eliminate it in all cases. Even if we can usually > handle it sanely in the kernel, userspace libraries / apps may not be so > robust. From my POV the core problem is clearly the jumping counter, > everything else discussed in this thread is just workarounds to paper > over it. I looked at this more; apparently the framecount reg on at least my machine is running way too fast, which triggered the behavior we talked about earlier. But given that we didn't use to track frame counts across VT switch (many drivers removed their IRQ handler, so the frame count wouldn't increase between LeaveVT and EnterVT) and things seemed ok, it's probably fine to just return 0 from the get_vblank_counter hook, and ignore frames we lost between interrupt/vblank off periods. Anyway I'll get that fixed one way or the other... So... if you feel strongly about the frame count cap, go ahead and push a change like you suggested above, otherwise you can ack the patch I posted earlier and everyone will be happy. :) -- Jesse Barnes, Intel Open Source Technology Center ------------------------------------------------------------------------------ Create and Deploy Rich Internet Apps outside the browser with Adobe(R)AIR(TM) software. With Adobe AIR, Ajax developers can use existing skills and code to build responsive, highly engaging applications that combine the power of local resources and data with the reach of the web. Download the Adobe AIR SDK and Ajax docs to start building applications today-http://p.sf.net/sfu/adobe-com -- _______________________________________________ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
