Re: [PATCH v2] timers: Clear must_forward_clk inside base lock

[Kohli, Gaurav]

On 8/2/2018 12:04 PM, Thomas Gleixner wrote:
> On Thu, 2 Aug 2018, Gaurav Kohli wrote:
> > Timer wheel base->must_forward_clock is indicating that
> > the base clock might be stale due to a long idle sleep.
> > The forwarding of base clock takes place in softirq of timer
>
> of the base clock takes place in the timer softirq ...
>
> > or when a timer is enqueued to base which is idle. While migrate
>
> to a base ..
>
> > timer from remote CPU to the new base which is idle, then
>
> See below.
>
> > following race can happen:
> >
> >    CPU0                                  CPU1
> >    run_timer_softirq                     timers_dead_cpu
> >
> > 					base = lock_timer_base(timer);
> >    base->must_forward_clk = false
> > 					if (base->must_forward_clk)
> > 				         forward(base); >>skip
> >
> > 					migrate_timer_list
>
> I don't know why you insist on migrate_timer_list() being part of the
> picture here.

Hi Thomas,

Thanks for comment.
I agree this can come with normal enqueue of timer as well, will make it 
more generic to avoid confuzion and upload new patch for same.

Regards
Gaurav


> It's only _ONE_ particular way to observe that issue. But it's not the only
> way. ANY remote enqueue which hits the situation on the other CPU (CPU0 in
> the example) has this problem. Tying it to migrate_timer_list() just
> because you observed it that way is actively misleading. Surely you can add
> a sentence that you observed it in that case, but that's supplemental
> information.
>
> Thanks,
>
> 	tglx

--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, 
Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.