Messages in this thread Patch in this message | | | Date | Wed, 17 Jan 2018 13:06:37 +0100 (CET) | From | Thomas Gleixner <> | Subject | Re: [PATCH] kernel: time: forward timer base before migrating timers |
| |
On Wed, 17 Jan 2018, Lingutla Chandrasekhar wrote:
> In case when timers are migrated to a CPU, after it exits > idle, but before timer base is forwarded, either from > run_timer_softirq()/mod_timer()/add_timer_on(), it's > possible that migrated timers are queued, based on older > clock value. This can cause delays in handling those timers. > > For example, consider below sequence of events: > > - CPU0 timer1 expires = 59969 and base->clk = 59131. So, > timer is queued at level 2, with next expiry for this timer > = 60032 (due to granularity addition). > - CPU1 enters idle @60007, with next timer expiry @60020. > - CPU1 exits idle. > - CPU0 is hotplugged at 60009, and timers are migrated to > CPU1, with new base->clk = 60007. timer1 is queued, > based on 60007 at level 0, for immediate handling (in > next timer softirq handling). > - CPU1's base->clk is forwarded to 60009, so, in next sched > timer interrupt, timer1 is not handled. > > The issue happens as timer wheel collects expired timers > starting from the current clk's index onwards, but migrated > timers, if enqueued, based on older clk value can result > in their index being less than clk's current index. > This can only happen if new base->clk is ahead of > timer->expires, resulting in timer being queued at > new base->clk's current index. > > Change-Id: Idbe737b346f00e6e7241b93181bbbd80871f1400
I asked that a gazillion of times now.
Please do not include your internal change id tags. They are completely useless for anybody else. Can you please make sure that anyone inside codeaurora knows?
> Signed-off-by: Neeraj Upadhyay <neeraju@codeaurora.org> > Signed-off-by: Lingutla Chandrasekhar <clingutla@codeaurora.org>
This SOB chain is wrong ....
> > diff --git a/kernel/time/timer.c b/kernel/time/timer.c > index 89a9e1b4264a..ae94aa97b5a9 100644 > --- a/kernel/time/timer.c > +++ b/kernel/time/timer.c > @@ -1886,6 +1886,11 @@ int timers_dead_cpu(unsigned int cpu) > raw_spin_lock_irq(&new_base->lock); > raw_spin_lock_nested(&old_base->lock, SINGLE_DEPTH_NESTING); > > + /* Before migrating timers, update new base clk to avoid > + * queueing timers based on older clock value. > + */
Multiline comments are
/* * First line * Second line */
Please do not use that retarded comment style above.
Thanks,
tglx
| |