On 8 July 2014 18:21, Frederic Weisbecker fweisbec@gmail.com wrote:
Makes sense, assuming I'm not missing something obvious.
Actually looking at the way it is written since the beginning, and the credentials of people that wrote it, I also have serious doubts about this stuff..
So be ready for a rant, I am :)
But you might want to add a WARN_ON_ONCE(!hrtimer_active(hrtimer)) on the end of these hrtimer_start common parts. In the end of __hrtimer_start_range_ns() maybe.
When I first read your reply, it looked a great idea. But thinking about it a bit more, it didn't look that great :)
Isn't it something like:
x = y; WARN_ON_ONCE(x != y);
i.e. we are adding a WARN where we are quite sure nothing can go wrong.
WARN()'s are probably good at places, where we don't expect things to happen in a certain way but they can still happen.
For example in the set_dev_mode() series, we had it on failure from ->set_dev_mode(), as platforms might try to return failures.
Having said that, I will still add this in a separate patch (the last one) just for testing. My trees are automated with kbuild bot and we can figure out if we really need to check for hrtimer_active().
This last one *shouldn't* be upstreamed :)
Let me know if you still want that WARN() to be there..
-- viresh