On Tue, Jun 16, 2015 at 05:30:19PM +0100, Chris Wilson wrote: > On Tue, Jun 16, 2015 at 06:21:53PM +0200, Daniel Vetter wrote: > > On Tue, Jun 16, 2015 at 01:10:33PM +0100, Chris Wilson wrote: > > > On Mon, Jun 15, 2015 at 06:34:51PM +0200, Daniel Vetter wrote: > > > > On Thu, Jun 11, 2015 at 09:01:08PM +0100, Chris Wilson wrote: > > > > > On Thu, Jun 11, 2015 at 07:14:28PM +0300, ville.syrjala@xxxxxxxxxxxxxxx wrote: > > > > > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > > > > > > > > > When the GPU gets reset __i915_wait_request() returns -EIO to the > > > > > > mmio flip worker. Currently we WARN whenever we get anything other > > > > > > than 0. Ignore the -EIO too since it's a perfectly normal thing > > > > > > to get during a GPU reset. > > > > > > > > > > Nak. I consider it is a bug in __i915_wait_request(). I am discussing > > > > > with Thomas Elf how to fix this wrt the next generation of individual > > > > > ring resets. > > > > > > > > We should only get an -EIO if the gpu is truly gone, but an -EAGAIN when > > > > the reset is ongoing. Neither is currently handled. For lockless users we > > > > probably want a version of wait_request which just dtrt (of waiting for > > > > the reset handler to complete without trying to grab the mutex and then > > > > returning). Or some other means of retrying. > > > > > > > > Returning -EIO from the low-level wait function still seems appropriate, > > > > but callers need to eat/handle it appropriately. WARN_ON isn't it here > > > > ofc. > > > > > > Bleh, a few years ago you decided not to take the EIO handling along the > > > call paths that don't care. > > > > > > I disagree. There are two classes of callers, those that care about > > > EIO/EAGAIN and those that simply want to know when the GPU is no longer > > > processing that request. That latter class is still popping up in > > > bugzilla with frozen displays. For the former, we actually only care > > > about backoff if we are holding the mutex - and that is only required > > > for EAGAIN. The only user that cares about EIO is throttle(). > > > > Hm, right now the design is that for non-interruptible designs we indeed > > return -EIO or -EAGAIN, but the reset handler will fix up outstanding > > flips. So I guess removing the WARN_ON here is indeed the right thing to > > do. We should probably change this once we have atomic (where the wait > > doesn't need a lock really, at least for async commits which is what > > matters here) and loop until completion. > > > > I'm still vary of eating -EIO in general since it's so hard to test all > > this for correctness. Maybe we need a __check_wedge which can return -EIO > > and a check_wedge which eats it. And then decide once for where to put > > special checks, probably just execbuf and throttle. > > Even execbuf really doesn't care. If the GPU didn't complete the earlier > request (principally for semaphore sw sync), it makes no difference for > us now. The content is either corrupt, or we bail when we spot the > wedged GPU upon writing to the ring. Reporting EIO because of an earlier > failure is a poor substitute for the async reset notification. But here > we still need EAGAIN backoff ofc. > > I really think eating EIO is the right thing to do in most circumstances > and is correct with the semantics of the callers. Well we once had the transparent sw fallback at least in the ddx for -EIO. Mesa never coped for obvious reasons, and given that a modern desktop can't survive with GL there's not all that much point any more. But still I think if the gpu is terminally dead we need to tell this to userspace somehow I think. What I'm unclear about is which ioctl that should be, and my assumption thus has been that it's execbuf. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
