On Wed, 9 Jul 2014, Viresh Kumar wrote:
So your patch series drops active hrtimer checks after adding it, according to your subject line.
Quite useeul to drop something after adding it, right?
hrtimer_start*() family never fails to enqueue a hrtimer to a clock-base. The only special case is when the hrtimer was in past. If it is getting enqueued to local CPUs's clock-base, we raise a softirq and exit, else we handle that on next interrupt on remote CPU.
At several places in the kernel, we try to make sure if hrtimer was added properly or not by calling hrtimer_active(), like:
hrtimer_start(timer, expires, mode); if (hrtimer_active(timer)) { /* Added successfully */ } else { /* Was added in the past */ }
As hrtimer_start*() never fails, hrtimer_active() is guaranteed to return '1'. So, there is no point calling hrtimer_active().
Wrong as usual.
It's a common pattern that short timeouts are given which lead to immediate expiry so the extra round through schedule is even more pointless than the extra check.
Aside of that it's a long discussed issue that we really should tell the caller right away that the timer was setup in the past and not enqueued at all.
That requires to fixup a few call sites, but that'd far more valuable than removing a few assumed to be pointless checks.
Thnaks,
tglx