Re: hardclock(9) could go.

["Mathew, Cherry G.*" <c%bow.st@localhost>]

>>>>> Martin Husemann <martin%duskware.de@localhost> writes:

    > On Wed, Mar 20, 2024 at 05:32:10AM +0000, Mathew, Cherry G.* wrote:
    >> I look forward to your comments and test results, if any.

    > I think moving heartbeat() to a callout context voids it's
    > purpose.

Thanks, Martin.

Please find attached an updated patch.

This moves everything, except heartbeat() and callout_hardclock() out of
hardclock().

I'm mainly interested in understanding how statclock() behaves wrt
perf stat collection. The scheduler seems to behave ok, in my testing,
but ymmv.

More testing and feedback will help, please!

Many Thanks,

-- 
Math/(~cherry)

diff -r 5949662bc15a sys/kern/kern_clock.c
--- a/sys/kern/kern_clock.c	Wed Mar 20 09:23:29 2024 +0000
+++ b/sys/kern/kern_clock.c	Wed Mar 20 10:31:03 2024 +0000
@@ -107,6 +107,9 @@
 
 static int sysctl_kern_clockrate(SYSCTLFN_PROTO);
 
+static callout_t clocktick_coh_var,  *clocktick_coh =
+  &clocktick_coh_var; /* Callout handle for clock tick */
+
 /*
  * Clock handling routines.
  *
@@ -222,6 +225,52 @@
 	return atomic_load_relaxed(&hardclock_ticks);
 }
 
+static void
+clocktick(void *clkfrm)
+{
+	struct lwp *l;
+	struct cpu_info *ci;
+
+	ci = curcpu();
+	l = ci->ci_onproc;
+
+	clockrnd_sample(&hardclockrnd);
+
+	if(clkfrm != NULL) {
+		struct clockframe *frame = clkfrm;
+
+		ptimer_tick(l, CLKF_USERMODE(frame));
+
+		/*
+		 * If no separate statistics clock is available, run it from here.
+		 */
+		if (stathz == 0)
+			statclock(frame);
+	} /* called from initclocks with NULL */
+
+	/*
+	 * If no separate schedclock is provided, call it here
+	 * at about 16 Hz.
+	 */
+	if (schedhz == 0) {
+		if ((int)(--ci->ci_schedstate.spc_schedticks) <= 0) {
+			schedclock(l);
+			ci->ci_schedstate.spc_schedticks = hardscheddiv;
+		}
+	}
+
+	if ((--ci->ci_schedstate.spc_ticks) <= 0)
+		sched_tick(ci);
+
+	if (CPU_IS_PRIMARY(ci)) {
+		atomic_store_relaxed(&hardclock_ticks,
+		    atomic_load_relaxed(&hardclock_ticks) + 1);
+		tc_ticktock();
+	}
+
+	callout_schedule(clocktick_coh, 1); /* Schedule next one */
+}
+     
 /*
  * Initialize clock frequencies and start both clocks running.
  */
@@ -254,6 +303,9 @@
 	 */
 	intr_timecounter.tc_frequency = hz;
 	tc_init(&intr_timecounter);
+	callout_init(clocktick_coh, CALLOUT_MPSAFE);
+	callout_setfunc(clocktick_coh, clocktick, NULL);
+	callout_schedule(clocktick_coh, 1); /* Schedule first one */
 
 	/*
 	 * Compute profhz and stathz, fix profhz if needed.
@@ -302,49 +354,16 @@
 void
 hardclock(struct clockframe *frame)
 {
-	struct lwp *l;
-	struct cpu_info *ci;
-
-	clockrnd_sample(&hardclockrnd);
-
-	ci = curcpu();
-	l = ci->ci_onproc;
-
-	ptimer_tick(l, CLKF_USERMODE(frame));
-
-	/*
-	 * If no separate statistics clock is available, run it from here.
-	 */
-	if (stathz == 0)
-		statclock(frame);
-	/*
-	 * If no separate schedclock is provided, call it here
-	 * at about 16 Hz.
-	 */
-	if (schedhz == 0) {
-		if ((int)(--ci->ci_schedstate.spc_schedticks) <= 0) {
-			schedclock(l);
-			ci->ci_schedstate.spc_schedticks = hardscheddiv;
-		}
-	}
-	if ((--ci->ci_schedstate.spc_ticks) <= 0)
-		sched_tick(ci);
-
-	if (CPU_IS_PRIMARY(ci)) {
-		atomic_store_relaxed(&hardclock_ticks,
-		    atomic_load_relaxed(&hardclock_ticks) + 1);
-		tc_ticktock();
-	}
-
 	/*
 	 * Make sure the CPUs and timecounter are making progress.
 	 */
 	heartbeat();
 
 	/*
-	 * Update real-time timeout queue.
+	 * Update real-time timeout queue, but update the frame first.
 	 */
-	callout_hardclock();
+	callout_setfunc(clocktick_coh, clocktick, frame);
+	callout_hardclock();  
 }
 
 /*