> Date: Mon, 19 May 2025 13:23:18 +0000
> From: Emmanuel Dreyfus <m...@netbsd.org>
> 
> On Mon, May 19, 2025 at 01:02:19PM +0000, Taylor R Campbell wrote:
> > Pass `-x nolibs' to dtrace -- this works around the bug where the Xen
> > bootloader doesn't pass the kernel's ctf sections to NetBSD (needs to
> > be fixed but I don't know how, would have to investigate how the Xen
> > loader works).
> 
> Tha works! Here is 60s of output. Do uou need more?
> https://dl.espci.fr/ticket/0e1a888502647a8f583dc6e8fb3b4c9e

Can you please try the attached patch and share any output from the
same dtrace script?
# HG changeset patch
# User Taylor R Campbell <riastr...@netbsd.org>
# Date 1747748859 0
#      Tue May 20 13:47:39 2025 +0000
# Branch trunk
# Node ID c9e7cfbadfe3534f37257ba74f9c01673ba2f124
# Parent  99071f28975be8f07e6ea8ae2f93b2e0e5e6c42e
# EXP-Topic riastradh-highrestimer
xen: Avoid playing catchup with the hardclock timer.

It's not clear this is worthwhile for anything except possibly the
emulated schedclock, which really shouldn't be running via hardclock
anyway -- should be done with separate high-resolution timer logic,
since Xen gives us a high-resolution timer.

Also:

1. Rearm the timer immediately, rather than after calling hardclock,
   so less time has elapsed since it fired -- this should reduce the
   chance that rearming the timer fails.

2. Add a dtrace probe for when we retry rearming the timer.

PR port-xen/NNNNN: ...
PR kern/MMMMM: (the hardclock catchup semantics PR)

diff -r 99071f28975b -r c9e7cfbadfe3 sys/arch/xen/xen/xen_clock.c
--- a/sys/arch/xen/xen/xen_clock.c      Tue May 20 13:43:03 2025 +0000
+++ b/sys/arch/xen/xen/xen_clock.c      Tue May 20 13:47:39 2025 +0000
@@ -127,6 +127,10 @@ SDT_PROBE_DEFINE3(sdt, xen, hardclock, j
     "uint64_t"/*last_systime_ns*/,
     "uint64_t"/*this_systime_ns*/,
     "uint64_t"/*nticks*/);
+SDT_PROBE_DEFINE3(sdt, xen, hardclock, rearm__failed,
+    "uint64_t"/*last_systime_ns*/,
+    "uint64_t"/*this_systime_ns*/,
+    "uint64_t"/*next_systime_ns*/);
 SDT_PROBE_DEFINE3(sdt, xen, hardclock, missed,
     "uint64_t"/*last_systime_ns*/,
     "uint64_t"/*this_systime_ns*/,
@@ -800,6 +804,7 @@ xen_timer_handler(void *cookie, struct c
 #if defined(XENPV)
        frame = NULL; /* We use values cached in curcpu()  */
 #endif
+
        /*
         * Find how many nanoseconds of Xen system time has elapsed
         * since the last hardclock tick.
@@ -822,12 +827,18 @@ xen_timer_handler(void *cookie, struct c
                 */
                ci->ci_xen_hardclock_systime_ns = last = now - ns_per_tick;
        }
+
+       /*
+        * Compute the time since the last tick.  Ideally, this should
+        * lie somewhere in [ns_per_tick, 2*ns_per_tick), but if the
+        * timer fired early it might lie in [0, ns_per_tick) and if
+        * the timer fired excessively late it might lie in
+        * [2*ns_per_tick, \infty).
+        */
        delta = now - last;
 
        /*
-        * Play hardclock catchup: run the hardclock timer as many
-        * times as appears necessary based on how much time has
-        * passed.
+        * Check to see if we ran late by a tick or more.
         */
        if (__predict_false(delta >= 2*ns_per_tick)) {
                SDT_PROBE3(sdt, xen, hardclock, jump,
@@ -848,36 +859,42 @@ xen_timer_handler(void *cookie, struct c
                            xen_timecounter.tc_counter_mask);
                        ci->ci_xen_timecounter_jump_evcnt.ev_count++;
                }
-               /* don't try to catch up more than one second at once */
-               if (delta > 1000000000UL)
-                       delta = 1000000000UL;
+
+               /*
+                * If delta \in [k*ns_per_tick, (k + 1)*ns_per_tick),
+                * set next := (k + 1)*ns_per_tick.  Note that
+                * rounddown(delta, ns_per_tick) = k*ns_per_tick, so we
+                * add one more to get (k + 1)*ns_per_tick.
+                */
+               next = last + rounddown(delta, ns_per_tick) + ns_per_tick;
+       } else {
+               next = last + 2*ns_per_tick;
        }
-       while (delta >= ns_per_tick) {
-               ci->ci_xen_hardclock_systime_ns += ns_per_tick;
-               delta -= ns_per_tick;
-               hardclock(frame);
-               if (__predict_false(delta >= ns_per_tick)) {
-                       SDT_PROBE3(sdt, xen, hardclock, missed,
-                           last, now, delta);
-                       ci->ci_xen_missed_hardclock_evcnt.ev_count++;
+
+       /*
+        * Re-arm the timer.  If it fails, the time may have already
+        * passed, so try one more tick later.  If that fails, panic.
+        */
+       error = HYPERVISOR_set_timer_op(next);
+       if (error) {
+               SDT_PROBE3(sdt, xen, hardclock, rearm__failed,
+                   last, now, next);
+               last = next;
+               next += roundup(xen_vcputime_systime_ns() - last,
+                   ns_per_tick);
+               next += ns_per_tick;
+               error = HYPERVISOR_set_timer_op(next);
+               if (error) {
+                       panic("failed to rearm Xen timer"
+                           " at %"PRIu64" ns or %"PRIu64" ns: %d",
+                           last, next, error);
                }
        }
 
        /*
-        * Re-arm the timer.  If it fails, it's probably because the
-        * time is in the past, possibly because we're in the
-        * process of catching up missed hardclock calls.
-        * In this case schedule a tick in the near future.
+        * Call hardclock once.
         */
-       next = ci->ci_xen_hardclock_systime_ns + ns_per_tick;
-       error = HYPERVISOR_set_timer_op(next);
-       if (error) {
-               next = xen_vcputime_systime_ns() + ns_per_tick / 2;
-               error = HYPERVISOR_set_timer_op(next);
-               if (error) {
-                       panic("failed to re-arm Xen timer %d", error);
-               }
-       }
+       hardclock(frame);
 
        /* Success!  */
        return 0;

Reply via email to