| Message ID | 20201008154651.1901126-12-arnd@arndb.de (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Clean up legacy clock tick users | expand |
Hi Arnd, On Thu, Oct 8, 2020 at 5:48 PM Arnd Bergmann <arnd@arndb.de> wrote: > There are no more users of xtime_update aside from legacy_timer_tick(), > so fold it into that function and remove the declaration. > > update_process_times() is now only called inside of the kernel/time/ > code, so the declaration can be moved there. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Thanks for your patch! Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org> The comment about xtime_update() in arch/ia64/kernel/time.c needs an update. Does the comment about update_process_times() in arch/openrisc/kernel/time.c needs an update, too? For Amiga and Atari/ARAnyM: Tested-by: Geert Uytterhoeven <geert@linux-m68k.org> Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Mon, Oct 12, 2020 at 3:16 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Thu, Oct 8, 2020 at 5:48 PM Arnd Bergmann <arnd@arndb.de> wrote: > > There are no more users of xtime_update aside from legacy_timer_tick(), > > so fold it into that function and remove the declaration. > > > > update_process_times() is now only called inside of the kernel/time/ > > code, so the declaration can be moved there. > > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > Thanks for your patch! > > Reviewed-by: Geert Uytterhoeven <geert@linux-m68k.org> > > The comment about xtime_update() in arch/ia64/kernel/time.c needs > an update. I think the correct action for ia64 would be to make it a proper clockevent driver with oneshot support, and remove the rest of this logic. I could try to rewrite the comment, but I tried not to touch that part since I don't understand the logic behind it. Maybe the ia64 maintainers can comment here why it even tries to skip a timer tick. Is there a danger of ending up with the timer irq permanently disabled if the timer_interrupt() function returns with the itm register in the past, or is this simply about not having too many interrupts in a row? > Does the comment about update_process_times() in > arch/openrisc/kernel/time.c needs an update, too? I think that one is still technically correct. Arnd
On Mon, Oct 12 2020 at 15:37, Arnd Bergmann wrote: > On Mon, Oct 12, 2020 at 3:16 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: >> On Thu, Oct 8, 2020 at 5:48 PM Arnd Bergmann <arnd@arndb.de> wrote: >> The comment about xtime_update() in arch/ia64/kernel/time.c needs >> an update. > > I think the correct action for ia64 would be to make it a > proper clockevent driver with oneshot support, and remove > the rest of this logic. Correct action would be to remove all of arch/ia64 :) > I could try to rewrite the comment, but I tried not to touch that > part since I don't understand the logic behind it. Maybe the > ia64 maintainers can comment here why it even tries to skip > a timer tick. Is there a danger of ending up with the timer irq > permanently disabled if the timer_interrupt() function returns > with the itm register in the past, or is this simply about not having > too many interrupts in a row? There was a comment in the initial ia64 code: * There is a race condition here: to be on the "safe" * side, we process timer ticks until itm.next is * ahead of the itc by at least half the timer * interval. This should give us enough time to set * the new itm value without losing a timer tick. The ITM (Interval Timer Match) register is raising an interrupt when the ITM value matches the ITC (Interval Timer Counter) register. If the new counter is already past the match then the timer interrupt will happen once ITC wrapped around and reaches the match value again. Might take ages for a 64bit counter to do that. :) IIRC, PXA had the same problem and HPET definitely has it as well. Seems Intel patented the concept of broken timers, but at least they listened when they proposed to implement the TSC deadline timer on x86 in the exact same way. See hpet_clkevt_set_next_event() for the gory details how to handle that correctly. Thanks, tglx
diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h index 3670cb1670ff..a8bef0ffcbdd 100644 --- a/include/linux/timekeeping.h +++ b/include/linux/timekeeping.h @@ -9,9 +9,7 @@ void timekeeping_init(void); extern int timekeeping_suspended; -/* Architecture timer tick functions: */ -extern void update_process_times(int user); -extern void xtime_update(unsigned long ticks); +/* Architecture timer tick functions */ extern void legacy_timer_tick(unsigned long ticks); /* diff --git a/kernel/time/tick-legacy.c b/kernel/time/tick-legacy.c index 73c5a0af4743..af225b32f5b3 100644 --- a/kernel/time/tick-legacy.c +++ b/kernel/time/tick-legacy.c @@ -10,10 +10,28 @@ #include "tick-internal.h" +/** + * legacy_timer_tick() - advances the timekeeping infrastructure + * @ticks: number of ticks, that have elapsed since the last call. + * + * This is used by platforms that have not been converted to + * generic clockevents. + * + * If 'ticks' is zero, the CPU is not handling timekeeping, so + * only perform process accounting and profiling. + * + * Must be called with interrupts disabled. + */ void legacy_timer_tick(unsigned long ticks) { - if (ticks) - xtime_update(ticks); + if (ticks) { + raw_spin_lock(&jiffies_lock); + write_seqcount_begin(&jiffies_seq); + do_timer(ticks); + write_seqcount_end(&jiffies_seq); + raw_spin_unlock(&jiffies_lock); + update_wall_time(); + } update_process_times(user_mode(get_irq_regs())); profile_tick(CPU_PROFILING); } diff --git a/kernel/time/timekeeping.c b/kernel/time/timekeeping.c index 6858a31364b6..2c7814411f83 100644 --- a/kernel/time/timekeeping.c +++ b/kernel/time/timekeeping.c @@ -2461,19 +2461,3 @@ void hardpps(const struct timespec64 *phase_ts, const struct timespec64 *raw_ts) } EXPORT_SYMBOL(hardpps); #endif /* CONFIG_NTP_PPS */ - -/** - * xtime_update() - advances the timekeeping infrastructure - * @ticks: number of ticks, that have elapsed since the last call. - * - * Must be called with interrupts disabled. - */ -void xtime_update(unsigned long ticks) -{ - raw_spin_lock(&jiffies_lock); - write_seqcount_begin(&jiffies_seq); - do_timer(ticks); - write_seqcount_end(&jiffies_seq); - raw_spin_unlock(&jiffies_lock); - update_wall_time(); -} diff --git a/kernel/time/timekeeping.h b/kernel/time/timekeeping.h index 099737f6f10c..d94b69c5b869 100644 --- a/kernel/time/timekeeping.h +++ b/kernel/time/timekeeping.h @@ -22,6 +22,7 @@ static inline int sched_clock_suspend(void) { return 0; } static inline void sched_clock_resume(void) { } #endif +extern void update_process_times(int user); extern void do_timer(unsigned long ticks); extern void update_wall_time(void);
There are no more users of xtime_update aside from legacy_timer_tick(), so fold it into that function and remove the declaration. update_process_times() is now only called inside of the kernel/time/ code, so the declaration can be moved there. Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- include/linux/timekeeping.h | 4 +--- kernel/time/tick-legacy.c | 22 ++++++++++++++++++++-- kernel/time/timekeeping.c | 16 ---------------- kernel/time/timekeeping.h | 1 + 4 files changed, 22 insertions(+), 21 deletions(-)