Re: iostat(8): implement periodic display with setitimer(2)

2022-12-28 Thread Todd C . Miller
On Wed, 28 Dec 2022 11:15:50 -0500, Scott Cheloha wrote:

> Neither are arbitrary.
>
> The current maximum of one hundred million seconds is based on the old
> nanosleep(2) limit prior to the 6.5 release.  It was a documented
> limit; see the manpage from the 6.4 release:
>
> http://man.openbsd.org/OpenBSD-6.4/nanosleep.2
>
> setitimer(2) accepts intervals up to UINT_MAX seconds.  This is not
> documented yet, but I have a rewrite for getitimer.2 in the works.
> The kernel enforces it in itimerfix():

I see.  OK millert@

 - todd



Re: iostat(8): implement periodic display with setitimer(2)

2022-12-28 Thread Scott Cheloha
On Wed, Dec 28, 2022 at 08:23:57AM -0700, Todd C. Miller wrote:
> On Wed, 28 Dec 2022 06:51:52 -0600, Scott Cheloha wrote:
> 
> > On Sat, Dec 10, 2022 at 08:09:15AM -0600, Scott Cheloha wrote:
> > > As with e.g. kstat(1), implementing a periodic display with
> > > nanosleep(2) causes the display interval to drift.  Doing it with
> > > setitimer(2) prevents drift.
> > > 
> > > ok?
> 
> Looks fine to me.  I don't see a reason to change the max interval
> though--both values appear arbitrary.

Neither are arbitrary.

The current maximum of one hundred million seconds is based on the old
nanosleep(2) limit prior to the 6.5 release.  It was a documented
limit; see the manpage from the 6.4 release:

http://man.openbsd.org/OpenBSD-6.4/nanosleep.2

setitimer(2) accepts intervals up to UINT_MAX seconds.  This is not
documented yet, but I have a rewrite for getitimer.2 in the works.
The kernel enforces it in itimerfix():

https://github.com/openbsd/src/blob/2b46a8cbba384f799321fc554d4d1c95563ea0a2/sys/kern/kern_time.c#L711



Re: iostat(8): implement periodic display with setitimer(2)

2022-12-28 Thread Todd C . Miller
On Wed, 28 Dec 2022 06:51:52 -0600, Scott Cheloha wrote:

> On Sat, Dec 10, 2022 at 08:09:15AM -0600, Scott Cheloha wrote:
> > As with e.g. kstat(1), implementing a periodic display with
> > nanosleep(2) causes the display interval to drift.  Doing it with
> > setitimer(2) prevents drift.
> > 
> > ok?

Looks fine to me.  I don't see a reason to change the max interval
though--both values appear arbitrary.

 - todd



Re: iostat(8): implement periodic display with setitimer(2)

2022-12-28 Thread Scott Cheloha
On Sat, Dec 10, 2022 at 08:09:15AM -0600, Scott Cheloha wrote:
> As with e.g. kstat(1), implementing a periodic display with
> nanosleep(2) causes the display interval to drift.  Doing it with
> setitimer(2) prevents drift.
> 
> ok?

Two week ping.

Index: iostat.c
===
RCS file: /cvs/src/usr.sbin/iostat/iostat.c,v
retrieving revision 1.45
diff -u -p -r1.45 iostat.c
--- iostat.c4 Dec 2022 23:50:51 -   1.45
+++ iostat.c28 Dec 2022 12:50:46 -
@@ -68,10 +68,12 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -85,7 +87,8 @@ extern intdk_ndrive;
 kvm_t *kd;
 char   *nlistf, *memf;
 
-inthz, reps, interval;
+inthz, reps;
+time_t interval;
 static int todo = 0;
 
 volatile sig_atomic_t wantheader;
@@ -100,6 +103,7 @@ volatile sig_atomic_t wantheader;
 static void cpustats(void);
 static void disk_stats(double);
 static void disk_stats2(double);
+static void sigalarm(int);
 static void sigheader(int);
 static void header(void);
 static void usage(void);
@@ -113,9 +117,10 @@ int dkinit(int);
 int
 main(int argc, char *argv[])
 {
+   struct itimerval itv;
const char *errstr;
+   sigset_t empty;
int ch, hdrcnt;
-   struct timespec ts;
 
while ((ch = getopt(argc, argv, "Cc:dDIM:N:Tw:")) != -1)
switch(ch) {
@@ -146,7 +151,7 @@ main(int argc, char *argv[])
todo |= SHOW_TTY;
break;
case 'w':
-   interval = strtonum(optarg, 1, 1, );
+   interval = strtonum(optarg, 1, UINT_MAX, );
if (errstr)
errx(1, "wait is %s", errstr);
break;
@@ -169,12 +174,20 @@ main(int argc, char *argv[])
dkreadstats();
selectdrives(argv);
 
-   ts.tv_sec = interval;
-   ts.tv_nsec = 0;
-
/* print a new header on sigcont */
signal(SIGCONT, sigheader);
 
+   if (interval != 0) {
+   if (signal(SIGALRM, sigalarm) == SIG_ERR)
+   err(1, "signal");
+   sigemptyset();
+   itv.it_value.tv_sec = interval;
+   itv.it_value.tv_usec = 0;
+   itv.it_interval = itv.it_value;
+   if (setitimer(ITIMER_REAL, , NULL) == -1)
+   err(1, "setitimer");
+   }
+
for (hdrcnt = 1;;) {
if (!--hdrcnt || wantheader) {
header();
@@ -188,7 +201,7 @@ main(int argc, char *argv[])
 
if (reps >= 0 && --reps <= 0)
break;
-   nanosleep(, NULL);
+   sigsuspend();
dkreadstats();
if (last.dk_ndrive != cur.dk_ndrive)
wantheader = 1;
@@ -196,6 +209,11 @@ main(int argc, char *argv[])
exit(0);
 }
 
+static void
+sigalarm(int signo)
+{
+}
+
 /*ARGSUSED*/
 static void
 sigheader(int signo)
@@ -421,7 +439,7 @@ selectdrives(char *argv[])
errx(1, "invalid interval or drive name: %s", *argv);
}
if (*argv) {
-   interval = strtonum(*argv, 1, 1, );
+   interval = strtonum(*argv, 1, UINT_MAX, );
if (errstr)
errx(1, "interval is %s", errstr);
if (*++argv) {



iostat(8): implement periodic display with setitimer(2)

2022-12-10 Thread Scott Cheloha
As with e.g. kstat(1), implementing a periodic display with
nanosleep(2) causes the display interval to drift.  Doing it with
setitimer(2) prevents drift.

ok?

Index: iostat.c
===
RCS file: /cvs/src/usr.sbin/iostat/iostat.c,v
retrieving revision 1.45
diff -u -p -r1.45 iostat.c
--- iostat.c4 Dec 2022 23:50:51 -   1.45
+++ iostat.c10 Dec 2022 14:00:50 -
@@ -68,10 +68,12 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -85,7 +87,8 @@ extern intdk_ndrive;
 kvm_t *kd;
 char   *nlistf, *memf;
 
-inthz, reps, interval;
+inthz, reps;
+time_t interval;
 static int todo = 0;
 
 volatile sig_atomic_t wantheader;
@@ -100,6 +103,7 @@ volatile sig_atomic_t wantheader;
 static void cpustats(void);
 static void disk_stats(double);
 static void disk_stats2(double);
+static void sigalarm(int);
 static void sigheader(int);
 static void header(void);
 static void usage(void);
@@ -113,9 +117,10 @@ int dkinit(int);
 int
 main(int argc, char *argv[])
 {
+   struct itimerval itv;
const char *errstr;
+   sigset_t empty;
int ch, hdrcnt;
-   struct timespec ts;
 
while ((ch = getopt(argc, argv, "Cc:dDIM:N:Tw:")) != -1)
switch(ch) {
@@ -146,7 +151,7 @@ main(int argc, char *argv[])
todo |= SHOW_TTY;
break;
case 'w':
-   interval = strtonum(optarg, 1, 1, );
+   interval = strtonum(optarg, 1, UINT_MAX, );
if (errstr)
errx(1, "wait is %s", errstr);
break;
@@ -169,12 +174,20 @@ main(int argc, char *argv[])
dkreadstats();
selectdrives(argv);
 
-   ts.tv_sec = interval;
-   ts.tv_nsec = 0;
-
/* print a new header on sigcont */
signal(SIGCONT, sigheader);
 
+   if (interval != 0) {
+   if (signal(SIGALRM, sigalarm) == SIG_ERR)
+   err(1, "signal");
+   sigemptyset();
+   itv.it_value.tv_sec = interval;
+   itv.it_value.tv_usec = 0;
+   itv.it_interval = itv.it_value;
+   if (setitimer(ITIMER_REAL, , NULL) == -1)
+   err(1, "setitimer");
+   }
+
for (hdrcnt = 1;;) {
if (!--hdrcnt || wantheader) {
header();
@@ -188,7 +201,7 @@ main(int argc, char *argv[])
 
if (reps >= 0 && --reps <= 0)
break;
-   nanosleep(, NULL);
+   sigsuspend();
dkreadstats();
if (last.dk_ndrive != cur.dk_ndrive)
wantheader = 1;
@@ -196,6 +209,11 @@ main(int argc, char *argv[])
exit(0);
 }
 
+static void
+sigalarm(int signo)
+{
+}
+
 /*ARGSUSED*/
 static void
 sigheader(int signo)
@@ -421,7 +439,7 @@ selectdrives(char *argv[])
errx(1, "invalid interval or drive name: %s", *argv);
}
if (*argv) {
-   interval = strtonum(*argv, 1, 1, );
+   interval = strtonum(*argv, 1, UINT_MAX, );
if (errstr)
errx(1, "interval is %s", errstr);
if (*++argv) {