On Fri, Aug 23, 2013 at 10:06 AM, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote: >> Yeah, I think this approach should work. A few comments: >> - I think we need a debugfs file to shut the safety quirk off - when >> testing on a machine where we actually miss interrupts it might be >> usful to get the warning output every time. > > I did consider a modparam. Exposing it would indeed necessitate some > protection against concurrent modificatin. > >> - I'd go for a new bool dev_priv->unreliable_seqno_signalling or so to >> avoid any races due to the rmw cycle you now do on dev_priv->quirks. > > There isn't a race during writing, as hangcheck should never be run > concurrently (or at least any concurrent calls filtered out at the start > of the function). The read side is inherently racey. It's more that thus far we've used dev_priv->quirks only for stuff which never changes, now we have a quirk which gets only set at runtime. It just feels conceptually wrong ;-) And if we add a 2nd such quirk it'll break a bit, hence why I'd prefer a distinct piece of state tracking >> - I'd have opted for a faster timeout of the fake irq, but one that rearms. > > Whoops, that was a mistake. The intention was to run at 100Hz, do you > want even faster? We could switch to a hrtimer and kill two birds with > one stone (as timer is singleshot only). I'd simply go with a 1 jiffies rearming timer. >> Also I'd love to be able to test all this (both the missed irq >> detection stuff and the fake irq) but I don't have a good idea right >> now ... So I guess we need to again hope that it won't break too >> quickly (since it eventually will break again). > > Disable the call to ring->get_irq. Perhaps the high word of > dev_priv->stop_rings? Yeah, something like stop_ring_irqs or so could work, maybe by wrapping the wake_up_all calls into a little helper like wake_up_ring which noops if the respective bit is set in stop_ring_irqs. But I'm not sure whether this is worth the fuzz. Otoh we've just proven that for untested code the question isn't if it breaks, but when ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
