Messages in this thread | | | From | Thomas Gleixner <> | Subject | Re: [RFC PATCH 03/10] timer: Simplify LVL_START() and calc_index() | Date | Thu, 02 Jul 2020 13:59:17 +0200 |
| |
Frederic Weisbecker <frederic@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?
> @@ -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.
Thanks,
tglx
| |