Messages in this thread |  | | From | Anna-Maria Behnsen <> | | Subject | Re: [PATCH v10 04/20] timers: Optimization for timer_base_try_to_set_idle() | | Date | Mon, 22 Jan 2024 12:48:21 +0100 |
| |
Frederic Weisbecker <frederic@kernel.org> writes:
> Le Mon, Jan 15, 2024 at 03:37:27PM +0100, Anna-Maria Behnsen a écrit : >> When tick is stopped also the timer base is_idle flag is set. When >> reentering the timer_base_try_to_set_idle() with the tick stopped, there is >> no need to check whether the timer base needs to be set idle again. When a >> timer was enqueued in the meantime, this is already handled by the >> tick_nohz_next_event() call which was executed before >> tick_nohz_stop_tick(). >> >> Signed-off-by: Anna-Maria Behnsen <anna-maria@linutronix.de> >> --- >> kernel/time/tick-sched.c | 2 +- >> kernel/time/timer.c | 11 ++++++++--- >> 2 files changed, 9 insertions(+), 4 deletions(-) >> >> diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c >> index c6223afc801f..27f1a2ae7f39 100644 >> --- a/kernel/time/tick-sched.c >> +++ b/kernel/time/tick-sched.c >> @@ -886,7 +886,7 @@ static void tick_nohz_stop_tick(struct tick_sched *ts, int cpu) >> struct clock_event_device *dev = __this_cpu_read(tick_cpu_device.evtdev); >> unsigned long basejiff = ts->last_jiffies; >> u64 basemono = ts->timer_expires_base; >> - bool timer_idle; >> + bool timer_idle = ts->tick_stopped; >> u64 expires; >> >> /* Make sure we won't be trying to stop it twice in a row. */ >> diff --git a/kernel/time/timer.c b/kernel/time/timer.c >> index 3a668060692e..2f69a485a070 100644 >> --- a/kernel/time/timer.c >> +++ b/kernel/time/timer.c >> @@ -1999,13 +1999,18 @@ u64 get_next_timer_interrupt(unsigned long basej, u64 basem) >> * timer_base_try_to_set_idle() - Try to set the idle state of the timer bases >> * @basej: base time jiffies >> * @basem: base time clock monotonic >> - * @idle: pointer to store the value of timer_base->is_idle >> + * @idle: pointer to store the value of timer_base->is_idle on return; >> + * *idle contains the information whether tick was already stopped >> * >> - * Returns the tick aligned clock monotonic time of the next pending >> - * timer or KTIME_MAX if no timer is pending. >> + * Returns the tick aligned clock monotonic time of the next pending timer or >> + * KTIME_MAX if no timer is pending. When tick was already stopped KTIME_MAX is >> + * returned as well. >> */ >> u64 timer_base_try_to_set_idle(unsigned long basej, u64 basem, bool *idle) >> { >> + if (*idle) >> + return KTIME_MAX; > > Ok now I see the reason behind the behavioural change. > > So either: > > * We remove the old behaviour consisting in clearing base->is_idle if the new > next timer is within a jiffy while the tick is stopped. But then the changelog > from the previous patch should state that and comments must be clarified. >
I would like to take 'either'. I thought the changelog already mentioned it. But maybe I have to make it more explicit. I'll go and rework the comments once more.
Thanks,
Anna-Maria
|  |