On Fri, 23 Oct 2020, Arnd Bergmann wrote:
On Sun, Oct 18, 2020 at 2:55 AM Finn Thain <fthain@xxxxxxxxxxxxxxxxxxx> wrote:
On Thu, 15 Oct 2020, Arnd Bergmann wrote:
On Thu, Oct 15, 2020 at 3:19 AM Finn Thain <fthain@xxxxxxxxxxxxxxxxxxx> wrote:
On Sat, 10 Oct 2020, Arnd Bergmann wrote:
That configuration still produces the same 5 KiB of bloat. I see that
kernel/time/Kconfig has this --
# Core internal switch. Selected by NO_HZ_COMMON / HIGH_RES_TIMERS. This is
# only related to the tick functionality. Oneshot clockevent devices
# are supported independent of this.
config TICK_ONESHOT
bool
But my question was really about both kinds of dead code (oneshot
device support and oneshot tick support). Anyway, after playing with
the code for a bit I don't see any easy way to reduce the growth in
text size.
Did you look more deeply into where those 5KB are? Is this just the code
in kernel/time/{clockevents,tick-common}.c and the added platform
specific bits, or is there something more?
What I did was to list the relevant functions using scripts/bloat-o-meter
and tried stubbing out some code related to oneshot clockevent devices. I
didn't find any low fruit and don't plan to pursue that without the help
of LTO.
I suppose the sysfs interface and the clockevents_update_freq() logic
are not really needed on m68k, but it wouldn't make much sense to split
those out either.
How does the 5KB bloat compare to the average bloat we get from one
release to the next? Geert has been collecting statistics for this.
Perhaps that 5 KB is justified by gaining the hrtimers feature... hard to
say; it's never been available on these platforms. I can see the value in
it though.
Yes, makes sense. I think the one remaining problem with the
periodic mode in this driver is that it can drop timer ticks when
interrupts are disabled for too long, while in oneshot mode there
may be a way to know how much time has passed since the last tick as
long as the counter does not overflow.
Is there any benefit from adopting a oneshot tick (rather than
periodic) if no clocksource is consulted when calculating the next
interval? (I'm assuming NO_HZ is not in use, for reasons discussed
below.)
If the clocksource does not set CLOCK_SOURCE_IS_CONTINOUS, the kernel
will keep using periodic timers and not allow hrtimers.
IIUC, when HIGH_RES_TIMERS=y, the kernel will enable hrtimers only if the
platform provides both a continuous clocksource device and a oneshot
clockevent device. However, the "jiffies" clocksource does not set
CLOCK_SOURCE_IS_CONTINOUS and neither does the one in
arch/arm/mach-rpc/time.c.
When HIGH_RES_TIMERS=n and NO_HZ_COMMON=n (which is presently the case for
all platforms with GENERIC_CLOCKEVENTS=n) there's no use for a oneshot
clockevent device, right?
It seems likely that NO_HZ_COMMON=n because the clocksources on these
platforms produce a periodic interrupt regardless (e.g.
clocksource/i8253.c, arm/rpc, m68k platform timers etc.).
Finally, HIGH_RES_TIMERS=n seems likely if the only clocksource is
unreliable (e.g. because it loses time due to interrupt priorities). There
may be a few of platforms in this category (Mac, Atari?).
I would agree that despite this oneshot mode is probably worse
overall for timekeeping if the register accesses introduce
systematic errors.
It probably has to be tried. But consulting a VIA clocksource on every
tick would be expensive on this platform, so if that was the only way
to avoid cumulative errors, I'd probably just stick with the periodic
tick.
I'm sure there is a tradeoff somewhere. Without hrtimers, some events
will take longer when they have to wait for the next tick, and using
NO_HZ_FULL can help help make things faster on some workloads.
Yes, such a tradeoff is discussed in drivers/iio/adc/ep93xx_adc.c.
But OTOH, Documentation/timers/timers-howto.rst says,
On slower systems, (embedded, OR perhaps a speed-stepped PC!) the
overhead of setting up the hrtimers for usleep *may* not be worth it
I guess it has to be tried.
...
The other 11 platforms in that category also have 'synthetic'
clocksources derived from a timer reload interrupt. In 3 cases, the
clocksource read method does not (or can not) check for a pending
counter reload interrupt. For these also, I see no practical
alternative to the approach you've taken in your RFC patch:
arch/m68k/68000/timers.c
arch/m68k/atari/time.c
arch/m68k/coldfire/timers.c
Agreed. It's possible there is a way, but I don't see one either.
For arch/m68k/68000/timers.c, I suppose we may be able to check for the
TMR1 bit in the Interrupt Status Register at 0xFFFFF30C or the COMP bit in
the Timer Status Register at 0xFFFFF60A. But testing that patch could be
difficult.
I expect that equivalent flags are available in Coldfire registers, making
it possible to improve upon mcftmr_read_clk() and m68328_read_clk() if
need be -- that is, if it turns out that the clocksource interrupt was
subject to higher priority IRQs that would slow down the clocksource or
defeat its monotonicity.
The other difficulty is a lack of hardware timers. There's only one timer
on MC68EZ328. On Atari, for now only Timer C is available though Michael
has said that it would be possible to free up Timer D. Some Coldfire chips
have only 2 timers and the second timer seems to be allocated to
profiling.
That leaves 8 platforms that have reliable clocksource devices which
should be able to provide an accurate reading even in the presence of
a dropped tick (due to drivers disabling interrupts for too long):
arch/arm/mach-rpc/time.c
arch/m68k/amiga/config.c
arch/m68k/bvme6000/config.c
arch/m68k/coldfire/sltimers.c
arch/m68k/hp300/time.c
arch/m68k/mac/via.c
arch/m68k/mvme147/config.c
arch/m68k/mvme16x/config.c
But is there any reason to adopt a oneshot tick on any of these platforms,
if NO_HZ won't eliminate the timer interrupt that's needed to run a
'synthetic' clocksource?
I would expect that these could be changed to behave more like
drivers/clocksource/i8253.c, which does support a clocksource in oneshot
mode.
I think you meant to write, "... support a clockevent device in oneshot
mode". I would agree.
Since Macs do have a spare hardware timer, I will attempt to convert
arch/m68k/mac/via.c to a oneshot clockevent, using your
GENERIC_CLOCKEVENTS patch series as a basis, and see what effect that has
in the NO_HZ_COMMON=n, HIGH_RES_TIMERS=y case.
Arnd
