Messages in this thread |  | | Date | Thu, 9 Jul 2020 14:17:35 +0200 (CEST) | From | Anna-Maria Behnsen <> | Subject | Re: [PATCH 1/9] timer: Move trigger_dyntick_cpu() to enqueue_timer() |
| |
Hi Frederic,
On Tue, 7 Jul 2020, Frederic Weisbecker wrote:
> Consolidate the code by calling trigger_dyntick_cpu() from > enqueue_timer() instead of calling it from all its callers.
Looks good, but maybe you could also update the comments in the code (see remarks below)?
> Signed-off-by: Frederic Weisbecker <frederic@kernel.org> > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Anna-Maria Gleixner <anna-maria@linutronix.de> > Cc: Juri Lelli <juri.lelli@redhat.com> > --- > kernel/time/timer.c | 46 +++++++++++++++++++-------------------------- > 1 file changed, 19 insertions(+), 27 deletions(-) > > diff --git a/kernel/time/timer.c b/kernel/time/timer.c > index 9a838d38dbe6..4c977df3610b 100644 > --- a/kernel/time/timer.c > +++ b/kernel/time/timer.c > @@ -596,13 +573,29 @@ trigger_dyntick_cpu(struct timer_base *base, struct timer_list *timer) > wake_up_nohz_cpu(base->cpu); > } > > -static void > -internal_add_timer(struct timer_base *base, struct timer_list *timer) > +/* > + * Enqueue the timer into the hash bucket, mark it pending in > + * the bitmap and store the index in the timer flags. > + */
enqueue_timer() also calls trigger_dyntick_cpu().
> +static void enqueue_timer(struct timer_base *base, struct timer_list *timer, > + unsigned int idx) > { > - __internal_add_timer(base, timer); > + hlist_add_head(&timer->entry, base->vectors + idx); > + __set_bit(idx, base->pending_map); > + timer_set_idx(timer, idx); > + > + trace_timer_start(timer, timer->expires, timer->flags); > trigger_dyntick_cpu(base, timer); > } > > +static void internal_add_timer(struct timer_base *base, struct timer_list *timer) > +{ > + unsigned int idx; > + > + idx = calc_wheel_index(timer->expires, base->clk); > + enqueue_timer(base, timer, idx); > +} > + > #ifdef CONFIG_DEBUG_OBJECTS_TIMERS > > static struct debug_obj_descr timer_debug_descr; > @@ -1059,7 +1052,6 @@ __mod_timer(struct timer_list *timer, unsigned long expires, unsigned int option > */
The comment above still mentions that trigger_dyntick_cpu() is called which is now part of enqueue_timer(). Might be a little confusing.
> if (idx != UINT_MAX && clk == base->clk) { > enqueue_timer(base, timer, idx); > - trigger_dyntick_cpu(base, timer); > } else { > internal_add_timer(base, timer); > }
Thanks,
Anna-Maria
|  |