On Thu, Apr 02, 2015 at 07:23:31PM +0530, Viresh Kumar wrote: > On 2 April 2015 at 19:17, Peter Zijlstra <[email protected]> wrote: > > On Thu, Apr 02, 2015 at 04:21:21PM +0530, Viresh Kumar wrote: > >> 'active_bases' indicates which clock-base have active timers. While it > >> is updated (almost) correctly, it is hardly used. Next commit will start > >> using it to make code more efficient, but before that we need to fix a > >> problem. > >> > >> While removing hrtimers, in __remove_hrtimer(): > >> - We first remove the hrtimer from the queue. > >> - Then reprogram clockevent device if required > >> (hrtimer_force_reprogram()). > >> - And then finally clear 'active_bases', if no more timers are pending > >> on the current clock base (from which we are removing the hrtimer). > >> > >> hrtimer_force_reprogram() needs to loop over all active clock bases to > >> find the next expiry event, and while doing so it will use > >> 'active_bases' (after next commit). And it will find the current base > >> active, as we haven't cleared it until now, even if current clock base > >> has no more hrtimers queued. > >> > >> To fix this issue, clear active_bases before calling > >> hrtimer_force_reprogram(). > > > > This is a small inefficiency right? Not an actual bug or anything. > > So, what's explained in this patch is a BUG, which isn't harming us today.
So then I'm not seeing how its a bug. Sure __hrtimer_get_next_event() will iterate all the bases again, and it will not skip the just empty one. But I don't see how that is anything but an inefficiency. By virtue of the base being empty it cannot find an event there, so its a pointless check. What am I missing? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
