Hi Ben,
I'm sorry for the very very late reply. Please do not jump to the conclusion that I do not care - I do! Just I am very busy, just as you.
On Wednesday 11 January 2012 01:40:58 pm Ben Skeggs wrote:
On Wed, 2012-01-11 at 11:17 +0100, Jean Delvare wrote:
In the commit message, you complain about the use of cond_resched() in i2c-algo-bit. You might as well be right, maybe it should be removed. It's been there pretty much forever (February 2002 at least) and while I can understand why it was put there, I would agree it isn't necessarily a good idea. Did you try removing it to see if it solved your problem?
I don't disagree, and I held off a long time before finally committing it to the nouveau repository due to not liking the duplication. My final decision to do it was admittedly due to having very very limited time and a *lot* of other things to work on. And this solution worked now.
The fact i2c-algo-bit can't be used from an atomic context was probably a convenient excuse I guess, but I did also figure there were good reasons for it and there'd likely be push-back at attempting to change that. Again, due to having loads of other things to do, I took the quick approach.
I don't think there actually is any good reason, and not only I wouldn't push back if someone wants to get rid of the call to cond_resched(), but I would even push for it to happen ;)
Now the question is, what do we replace it with? Nothing? cpu_relax()? yield()? udelay(<small value>)?
I guess yield() would have the same property of making the code not callable from atomic context. Not having anything might be harsh from a CPU and I/O load perspective, as it would be a tight loop lasting for several milliseconds. So this leaves us with cpu_relax() and udelay(). Any preference?
(...) I'd be more than happy to have these issues (nvs300 and running from an atomic context) solved in i2c-algo-bit, and will gladly revert the nouveau changes once it is. Let me know how I can help you with that.
Incidentally I just sent a fix upstream for a bug reported by Ville Syrjälä in i2c-algo-bit, that would cause spurious timeouts on heavy CPU load. I am curious if it would have helped in your case. Please take a look:
http://marc.info/?l=linux-kernel&m=133182203301053&w=2
And if you think it could help, I would appreciate if you could give it a try with the nouveau driver and see if it actually does.
Thanks,