On Thu, Jul 02, 2020 at 01:59:17PM +0200, Thomas Gleixner wrote: > Frederic Weisbecker <frede...@kernel.org> writes: > > LVL_START() makes the first index of a level to start with what would be > > the value of all bits set of the previous level. > > > > For example level 1 starts at 63 instead of 64. > > > > To cope with that, calc_index() always adds one offset for the level > > granularity to the expiry passed in parameter. > > > > Yet there is no apparent reason for such fixups so simplify the whole > > thing. > > You sure?
No :o) > > > @@ -158,7 +158,7 @@ EXPORT_SYMBOL(jiffies_64); > > * The time start value for each level to select the bucket at enqueue > > * time. > > */ > > -#define LVL_START(n) ((LVL_SIZE - 1) << (((n) - 1) * LVL_CLK_SHIFT)) > > +#define LVL_START(n) (LVL_SIZE << (((n) - 1) * LVL_CLK_SHIFT)) > > > /* Size of each clock level */ > > #define LVL_BITS 6 > > @@ -489,7 +489,7 @@ static inline void timer_set_idx(struct timer_list > > *timer, unsigned int idx) > > */ > > static inline unsigned calc_index(unsigned expires, unsigned lvl) > > { > > - expires = (expires + LVL_GRAN(lvl)) >> LVL_SHIFT(lvl); > > + expires >>= LVL_SHIFT(lvl); > > return LVL_OFFS(lvl) + (expires & LVL_MASK); > > } > > So with that you move the expiry of each timer one jiffie ahead vs. the > original code, which violates the guarantee that a timer sleeps at least > for one jiffie for real and not measured in jiffies. > > base->clk = 1 > jiffies = 0 > local_irq_disable() > -> timer interrupt is raised in HW > timer->expires = 1 > add_timer(timer) > ---> index == 1 > local_irq_enable() > timer interrupt > jiffies++; > softirq() > expire(timer); > > So the off by one has a reason. Fair enough. I didn't know about the one jiffy sleep guarantee. I'll convert this patch to a comment explaining that off by one, using your above example. Thanks!