On Wed, Jan 04, 2012 at 01:28:28PM +0000, Alan Cox wrote:
unnusual contexts). I'm not sure whether your proposed instrumentation is really worth it though - in i915-land we don't bother with this. But maybe
In i915 land it's specific code paths so effectively it is marked up. We don't do it for every random delay in all the connectors and other bits. The bits of code believing they are safe use sleeping locks and we'll get spewage if that breaks. Ditto at this point I hope gma500 although I would not be surpised to find ones I still need to fix from being mdelay.
The atomic context check is in the _wait_for macro in intel_drv.h which is used all over the place. Like in the wait_for_vblank function. So I don't think you can use i915.ko as a shiny beacon of how to do it ;-)
with the firmware provided and randomly b0rked atombios tables it might make sense to add the WARN_ON_ONCE you've proposed in the other mail
I think we need it because it can be added by firmware, it could be silently done and the atombios paths cover so many different things beyond modesetting using that exact same function we need the mark up.
If some vendor puts a 100ms delay in a connector related hotplug check we are going to have a horrible time debugging 'bugzilla #zillion, 'poor performance in OpenGL game with Radeon foozillion'
Well, hotplug is alreay a giant pain because of the single lock to rule them all design of the current kms code: Long delays already stall everything else (well, cursor updates and pageflips) and we have tons of bugreports that complain about it. In a sense your example undermines my point that we have lower hanger fruit to fix all over the place already ...
But as I've said I like the WARN_ON you want to add, but otoh hand think it shouldn't be used to stall the small&hackish fix of adding the relevant in_atomic checks. -Daniel