On Thu, Sep 4, 2014 at 2:30 PM, John Stultz <john.stu...@linaro.org> wrote: > On Thu, Sep 4, 2014 at 2:17 PM, Andrew Hunter <a...@google.com> wrote: >> On Wed, Sep 3, 2014 at 5:06 PM, John Stultz <john.stu...@linaro.org> wrote: >>> Maybe with the next version of the patch, before you get into the >>> unwinding the math, you might practically describe what is broken, >>> then explain how its broken. >>> >>> My quick read here is that we're converting a timespec -> jiffies, and >>> in doing so we round up by one jiffy. >>> >>> This seems actually perfectly normal, as we usually end up rounding up >>> by a jiffy in many cases since we don't want to undershoot any >>> timeout, and we're always allowed return later then specified. >> >> Well, yes, timeouts can be longer than specified, but what you said >> technically applies just as much to code that arbitrarily multiplies >> jiffies by 10 before returning, no? :) >> >> The problem isn't the rounding, it's that the rounding is _wrong_: I'm >> fine rounding 10100usec / (1000 usec/jiffie) = 11 jiffies. The current >> code rounds 10000usec / (1000 usec/jiffies) to 11. I've rewritten the >> description to make this clearer. > > Ok. Very much appreciated! > >>>> In particular, with HZ=1000, we consistently computed that 10000 usec >>>> was 11 jiffies; the same was true for any exact multiple of >>>> TICK_NSEC. This is obviously bad as a general rule, and caused >>>> observable user problems with setitimer() at the very least: >>>> >>>> setitimer(ITIMER_PROF, &val, NULL); >>>> setitimer(ITIMER_PROF, NULL, &val); >>>> >>>> would actually add a tick to val! >>> >>> So this looks like an issue. Since we convert and store the internal >>> representation in jiffies, when we pull it back out, we get the >>> rounded up value, which is larger then the timespec value originally >>> submitted. This is really the core issue, correct? >> >> For the particular user bug reported to me, yes, this was the core >> issue: some code that stopped and restarted an itimer found the >> interval growing by 1ms each time. But again, it wasn't that it was >> rounded: if we initially passed e.g. 10500 usec and got back 11000, >> that'd be annoying but workable, because if we then went through >> another cycle of enabling/disabling itimer, we'd set it to 11000 usec >> and get back 11000 again. What we have now instead adds a full jiffie >> _every time_. > > Ah, ok. This part is key to understanding the problem. Thanks for > clarifying this. > > This seems to be a quite old bug.. Do you think this is needed for -stable?
Seems reasonable to me. > > thanks > -john -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/