Re: moncontrol(3): remove hertz()

[Philip Guenther]

On Mon, Dec 6, 2021 at 10:30 AM Scott Cheloha 
wrote:

> In the moncontrol(3) code we have this fallback function, hertz().
> The idea is to use an undocumented side effect of setitimer(2) to
> determine the width of the hardclock(9) tick.
>
> hertz() has a number of problems:
>


> So, I want to get rid of hertz() and replace it with nothing.  This is
> an extreme edge case.  sysctl(2) has to fail.  I do not think that is
> possible here, outside of stack corruption.
>

I agree: sysctl(KERN_CLOCKRATE) isn't blocked by pledge(2), so how would
they manage to break it?

(I'm not even sure the failover from profhz to hz should be kept: the
kernel does that itself in initclocks(9) so if profhz is 0 then hz is too,
sysctl(2) failed, and your CPU is on fire.)

ok guenther@

moncontrol(3): remove hertz()

[Scott Cheloha]

In the moncontrol(3) code we have this fallback function, hertz().
The idea is to use an undocumented side effect of setitimer(2) to
determine the width of the hardclock(9) tick.

hertz() has a number of problems:

1. Per the setitimer(2) standard, the value of it_interval is ignored
   if it_value is zero.  So, I broke this code over a year ago.  Whoops.

2. hertz() silently cancels any running ITIMER_REAL timer and does not
   attempt to restart the timer when it is finished.  This is not documented.

3. hertz() doesn't cancel the timer it starts after it's done computing
   the hardclock(9) width.  So the application gets a SIGALRM later.

We could fix (3).  But (2) is a race, you can't fix it, not perfectly.
And I don't want to roll back (1) for the sake of this crummy code.

So, I want to get rid of hertz() and replace it with nothing.  This is
an extreme edge case.  sysctl(2) has to fail.  I do not think that is
possible here, outside of stack corruption.

Objections?

Index: gmon.c
===
RCS file: /cvs/src/lib/libc/gmon/gmon.c,v
retrieving revision 1.32
diff -u -p -r1.32 gmon.c
--- gmon.c  12 Oct 2020 22:08:33 -  1.32
+++ gmon.c  6 Dec 2021 18:23:12 -
@@ -50,7 +50,6 @@ static ints_scale;
 
 PROTO_NORMAL(moncontrol);
 PROTO_DEPRECATED(monstartup);
-static int hertz(void);
 
 void
 monstartup(u_long lowpc, u_long highpc)
@@ -159,17 +158,15 @@ _mcleanup(void)
if (p->state == GMON_PROF_ERROR)
ERR("_mcleanup: tos overflow\n");
 
+   /*
+* There is nothing we can do if sysctl(2) fails or if
+* clockinfo.hz is unset.
+*/
size = sizeof(clockinfo);
if (sysctl(mib, 2, , , NULL, 0) == -1) {
-   /*
-* Best guess
-*/
-   clockinfo.profhz = hertz();
+   clockinfo.profhz = 0;
} else if (clockinfo.profhz == 0) {
-   if (clockinfo.hz != 0)
-   clockinfo.profhz = clockinfo.hz;
-   else
-   clockinfo.profhz = hertz();
+   clockinfo.profhz = clockinfo.hz;/* best guess */
}
 
moncontrol(0);
@@ -304,23 +301,3 @@ moncontrol(int mode)
}
 }
 DEF_WEAK(moncontrol);
-
-/*
- * discover the tick frequency of the machine
- * if something goes wrong, we return 0, an impossible hertz.
- */
-static int
-hertz(void)
-{
-   struct itimerval tim;
-
-   tim.it_interval.tv_sec = 0;
-   tim.it_interval.tv_usec = 1;
-   tim.it_value.tv_sec = 0;
-   tim.it_value.tv_usec = 0;
-   setitimer(ITIMER_REAL, , 0);
-   setitimer(ITIMER_REAL, 0, );
-   if (tim.it_interval.tv_usec < 2)
-   return(0);
-   return (100 / tim.it_interval.tv_usec);
-}