On Wed, Apr 24, 2013 at 10:35:16AM +0100, Chris Wilson wrote: > On Wed, Apr 24, 2013 at 11:19:20AM +0200, Daniel Vetter wrote: > > Our rps code relies on the interrupts being off to prevent re-arming > > of the work items at inopportune moments. > > > > Also drop the redundant cancel_work for the main rps work, > > disable_gt_powersave already takes care of that. > > > > Finally add a WARN_ON to ensure we obey that piece of ordering > > constraint. Long term I want to lock down the setup/teardown code in a > > similar way to how we painstakingly check modeset sequence constraints > > already. > > Reminds me, did we every get around to flushing our deferred hw tasks > upon suspend? I've a funny feeling that is still missing. I think with the unification between suspend and driver unload we're mostly there. Like I've said I think it's time to encode all these constraints properly into debug asserts, but the problem I have is that I don't have a good idea to encode things like "this work item is now guaranteed to never get re-armed again". There's the object debug code in the kernel, but that only fires when the work/timer is actually armed. Since a lot of the work stuff happens rarely or is not delayed that's not really good enough ... Ideas for this very much welcome, since the lack of these self-checks makes any attempt to solve this for real futile. devres.c might help a bit in preventing resource leaks and tidy up the code, but it doesn't help at all for correct ordering and cleanup of async workers/timers. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch