Re: clock event deviceâs next_event

[Sodagudi Prasad]

On 2017-10-24 00:23, Thomas Gleixner wrote:
> On Mon, 23 Oct 2017, Sodagudi Prasad wrote:
>
> > Hi Viresh and Thomas,
> >
> > In the functions tick_nohz_stop_sched_tick(), when expires = KTIME_MAX 
> > we are
> > canceling the tick_sched_timer timer but we are not updating the clock 
> > event
> > deviceâs next_event to KTIME_MAX.
> > Due to that broadcast deviceâs next_event is not programmed properly 
> > and
> > resulting unnecessary wakeups for this cpu.
> >
> >         /*
> >          * If the expiration time == KTIME_MAX, then we simply stop
> >          * the tick timer.
> >          */
> >         if (unlikely(expires == KTIME_MAX)) {
> >                 if (ts->nohz_mode == NOHZ_MODE_HIGHRES)
> >                         hrtimer_cancel(&ts->sched_timer);
> >                 goto out;
> >         }
>
> Right, because this code does not have access to the broadcast device 
> at
> all. It doesn't even know and care about it.

Some of the drivers in cpuidle frame works are using 
tick_nohz_get_sleep_length API
to find maximum idle/sleep time from ts->sleep_length.

And also the estimated sleep length until the next timer in 
tick_nohz_stop_sched_tick
not getting updated at the end.

> > After digging further, I see that following call flow is updating
> > tick_cpu_device state to shutdown state but clock event device  
> > next_event is
> > not updated to KTIME_MAX.
> > hrtimer_cancel -> __remove_hrtimer -> hrtimer_force_reprogram ->
> > tick_program_event.
> >
> > int tick_program_event(ktime_t expires, int force)
> > {
> >         struct clock_event_device *dev =
> > __this_cpu_read(tick_cpu_device.evtdev);
> >
> >         if (unlikely(expires == KTIME_MAX)) {
> >                 /*
> >                  * We don't need the clock event device any more, stop 
> > it.
> >                  */
> >                 clockevents_switch_state(dev,
> > CLOCK_EVT_STATE_ONESHOT_STOPPED);
> >                 return 0;
> >         }
> > In the above tick_program_event() function clock event deviceâs 
> > next_event is
> > not getting updated as clockevents_program_event() function not called 
> > after
> > state update.
>
> If the device is shutdown, then next_event does not matter. But yes, 
> for
> consistency reasons we could set it to KTIME_MAX.
Thanks tglx for your quick reply on this and I will send patch for the 
same.

> Thanks,
>
> 	tglx

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum,
Linux Foundation Collaborative Project