[01/13] timekeeping: add CONFIG_LEGACY_TIMER_TICK
diff mbox series

Message ID 20201008154651.1901126-2-arnd@arndb.de
State New, archived
Headers show
Series
  • Clean up legacy clock tick users
Related show

Commit Message

Arnd Bergmann Oct. 8, 2020, 3:46 p.m. UTC
All platforms that currently do not use generic clockevents roughly call
the same set of functions in their timer interrupts: xtime_update(),
update_process_times() and profile_tick(), sometimes in a different
sequence.

Add a helper function that performs all three of them, to make the
callers more uniform and simplify the interface.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 include/linux/timekeeping.h |  1 +
 kernel/time/Kconfig         |  7 +++++++
 kernel/time/Makefile        |  1 +
 kernel/time/tick-legacy.c   | 19 +++++++++++++++++++
 4 files changed, 28 insertions(+)
 create mode 100644 kernel/time/tick-legacy.c

Comments

Finn Thain Oct. 9, 2020, 10:18 p.m. UTC | #1
Hi Arnd,

On Thu, 8 Oct 2020, Arnd Bergmann wrote:

> All platforms that currently do not use generic clockevents roughly call
> the same set of functions in their timer interrupts: xtime_update(),
> update_process_times() and profile_tick(), sometimes in a different
> sequence.
> 
> Add a helper function that performs all three of them, to make the
> callers more uniform and simplify the interface.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  include/linux/timekeeping.h |  1 +
>  kernel/time/Kconfig         |  7 +++++++
>  kernel/time/Makefile        |  1 +
>  kernel/time/tick-legacy.c   | 19 +++++++++++++++++++
>  4 files changed, 28 insertions(+)
>  create mode 100644 kernel/time/tick-legacy.c
> 
> diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h
> index 7f7e4a3f4394..3670cb1670ff 100644
> --- a/include/linux/timekeeping.h
> +++ b/include/linux/timekeeping.h
> @@ -12,6 +12,7 @@ extern int timekeeping_suspended;
>  /* Architecture timer tick functions: */
>  extern void update_process_times(int user);
>  extern void xtime_update(unsigned long ticks);
> +extern void legacy_timer_tick(unsigned long ticks);
>  
>  /*
>   * Get and set timeofday
> diff --git a/kernel/time/Kconfig b/kernel/time/Kconfig
> index a09b1d61df6a..f2b0cfeade47 100644
> --- a/kernel/time/Kconfig
> +++ b/kernel/time/Kconfig
> @@ -61,6 +61,13 @@ config POSIX_CPU_TIMERS_TASK_WORK
>  	bool
>  	default y if POSIX_TIMERS && HAVE_POSIX_CPU_TIMERS_TASK_WORK
>  
> +config LEGACY_TIMER_TICK
> +	bool
> +	help
> +	  The legacy timer tick helper is used by platforms that
> +	  lack support for the generic clockevent framework.
> +	  New platforms should use generic clockevents instead.
> +
>  if GENERIC_CLOCKEVENTS
>  menu "Timers subsystem"
>  
> diff --git a/kernel/time/Makefile b/kernel/time/Makefile
> index c8f00168afe8..1fb1c1ef6a19 100644
> --- a/kernel/time/Makefile
> +++ b/kernel/time/Makefile
> @@ -16,6 +16,7 @@ ifeq ($(CONFIG_GENERIC_CLOCKEVENTS_BROADCAST),y)
>  endif
>  obj-$(CONFIG_GENERIC_SCHED_CLOCK)		+= sched_clock.o
>  obj-$(CONFIG_TICK_ONESHOT)			+= tick-oneshot.o tick-sched.o
> +obj-$(CONFIG_LEGACY_TIMER_TICK)			+= tick-legacy.o
>  obj-$(CONFIG_HAVE_GENERIC_VDSO)			+= vsyscall.o
>  obj-$(CONFIG_DEBUG_FS)				+= timekeeping_debug.o
>  obj-$(CONFIG_TEST_UDELAY)			+= test_udelay.o
> diff --git a/kernel/time/tick-legacy.c b/kernel/time/tick-legacy.c
> new file mode 100644
> index 000000000000..73c5a0af4743
> --- /dev/null
> +++ b/kernel/time/tick-legacy.c
> @@ -0,0 +1,19 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Timer tick function for architectures that lack generic clockevents,
> + * consolidated here from m68k/ia64/parisc/arm.
> + */
> +
> +#include <linux/irq.h>
> +#include <linux/profile.h>
> +#include <linux/timekeeper_internal.h>
> +
> +#include "tick-internal.h"
> +
> +void legacy_timer_tick(unsigned long ticks)
> +{
> +	if (ticks)
> +		xtime_update(ticks);
> +	update_process_times(user_mode(get_irq_regs()));
> +	profile_tick(CPU_PROFILING);
> +}
> 

It's good to see this code refactored in this way because, as well as 
de-duplication, it reveals the logic that's common to the relevant 
platforms and may shed some light on the need for that logic.

Yet it's not clear to me that the clockevents framework is able to replace 
that logic on all of the affected hardware. I suppose it remains to be 
seen.

I hate to quibble about naming, but you seem to be using "legacy" here to 
mean "deprecated" (?) Is it a good idea to prepend such adjectives to 
symbol names?

IMO, the term "legacy" is redundant in this context. That term covers a 
large portion of kernel code, a large number of hardware features in 
current silicon, a large portion of the userspace ABI, a large number of 
production Linux systems, probably all "Unix" systems, etc.

As a corollary, cutting edge ("non-legacy") code is often kept out of open 
source projects by the owners of the intellectual property rights.
Arnd Bergmann Oct. 10, 2020, 8:31 p.m. UTC | #2
On Sat, Oct 10, 2020 at 12:18 AM Finn Thain <fthain@telegraphics.com.au> wrote:
> On Thu, 8 Oct 2020, Arnd Bergmann wrote:
>
> It's good to see this code refactored in this way because, as well as
> de-duplication, it reveals the logic that's common to the relevant
> platforms and may shed some light on the need for that logic.
>
> Yet it's not clear to me that the clockevents framework is able to replace
> that logic on all of the affected hardware. I suppose it remains to be
> seen.

I suspect that the change I did for one platform in patch 13/13 could be
duplicated for all 16 platforms, adding lots of trivial clockevent drivers that
only support periodic ticks, but any platform that can instead support
oneshot timers should probably do that, or it won't provide any better
behavior.

What do others think we should do here?

> As a corollary, cutting edge ("non-legacy") code is often kept out of open
> source projects by the owners of the intellectual property rights.

I'm happy to change the name in any way if you have a suggestion
that the clock event maintainers (Daniel and Thomas) like.

      Arnd
Geert Uytterhoeven Oct. 12, 2020, 1:14 p.m. UTC | #3
On Thu, Oct 8, 2020 at 5:47 PM Arnd Bergmann <arnd@arndb.de> wrote:
> All platforms that currently do not use generic clockevents roughly call
> the same set of functions in their timer interrupts: xtime_update(),
> update_process_times() and profile_tick(), sometimes in a different
> sequence.
>
> Add a helper function that performs all three of them, to make the
> callers more uniform and simplify the interface.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Reviewed-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

Patch
diff mbox series

diff --git a/include/linux/timekeeping.h b/include/linux/timekeeping.h
index 7f7e4a3f4394..3670cb1670ff 100644
--- a/include/linux/timekeeping.h
+++ b/include/linux/timekeeping.h
@@ -12,6 +12,7 @@  extern int timekeeping_suspended;
 /* Architecture timer tick functions: */
 extern void update_process_times(int user);
 extern void xtime_update(unsigned long ticks);
+extern void legacy_timer_tick(unsigned long ticks);
 
 /*
  * Get and set timeofday
diff --git a/kernel/time/Kconfig b/kernel/time/Kconfig
index a09b1d61df6a..f2b0cfeade47 100644
--- a/kernel/time/Kconfig
+++ b/kernel/time/Kconfig
@@ -61,6 +61,13 @@  config POSIX_CPU_TIMERS_TASK_WORK
 	bool
 	default y if POSIX_TIMERS && HAVE_POSIX_CPU_TIMERS_TASK_WORK
 
+config LEGACY_TIMER_TICK
+	bool
+	help
+	  The legacy timer tick helper is used by platforms that
+	  lack support for the generic clockevent framework.
+	  New platforms should use generic clockevents instead.
+
 if GENERIC_CLOCKEVENTS
 menu "Timers subsystem"
 
diff --git a/kernel/time/Makefile b/kernel/time/Makefile
index c8f00168afe8..1fb1c1ef6a19 100644
--- a/kernel/time/Makefile
+++ b/kernel/time/Makefile
@@ -16,6 +16,7 @@  ifeq ($(CONFIG_GENERIC_CLOCKEVENTS_BROADCAST),y)
 endif
 obj-$(CONFIG_GENERIC_SCHED_CLOCK)		+= sched_clock.o
 obj-$(CONFIG_TICK_ONESHOT)			+= tick-oneshot.o tick-sched.o
+obj-$(CONFIG_LEGACY_TIMER_TICK)			+= tick-legacy.o
 obj-$(CONFIG_HAVE_GENERIC_VDSO)			+= vsyscall.o
 obj-$(CONFIG_DEBUG_FS)				+= timekeeping_debug.o
 obj-$(CONFIG_TEST_UDELAY)			+= test_udelay.o
diff --git a/kernel/time/tick-legacy.c b/kernel/time/tick-legacy.c
new file mode 100644
index 000000000000..73c5a0af4743
--- /dev/null
+++ b/kernel/time/tick-legacy.c
@@ -0,0 +1,19 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Timer tick function for architectures that lack generic clockevents,
+ * consolidated here from m68k/ia64/parisc/arm.
+ */
+
+#include <linux/irq.h>
+#include <linux/profile.h>
+#include <linux/timekeeper_internal.h>
+
+#include "tick-internal.h"
+
+void legacy_timer_tick(unsigned long ticks)
+{
+	if (ticks)
+		xtime_update(ticks);
+	update_process_times(user_mode(get_irq_regs()));
+	profile_tick(CPU_PROFILING);
+}