Re: clock event device’s next_event

[Sodagudi Prasad <psodagud@codeaurora.org>]

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