On Thu, Apr 28, 2022 at 08:54:02PM -0500, Scott Cheloha wrote:
> On Thu, Sep 17, 2020 at 06:29:48PM -0500, Scott Cheloha wrote:
> > [...]
> >
> > Using nanosleep(2) to print the stats periodically causes the period
> > to drift. If you use setitimer(2) it won't drift.
> >
> > ok?
>
> 19 month bump and rebase.
>
> I have updated the patch according to input from kn@.
>
> Once again, using nanosleep(2) here to print the stats periodically is
> flawed. The period will drift. Using setitimer(2)/sigsuspend(2) is
> better.
>
> While here:
>
> - We don't need the hundred million second upper bound anymore. Just
> cap the wait at UINT_MAX seconds.
>
> - Use the idiomatic strtonum(3) error message format, it works here.
>
> ok?
I would prefer to block the alarm signal with sigprocmask(2) and
only catch it during sigsuspend(2). Although the timeout should
only happen while we sleep, blocking signals while we don't expect
them, gives me a better feeling.
Please check the error code of signal(3).
otherwise diff looks good to me
> Index: kstat.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/kstat/kstat.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 kstat.c
> --- kstat.c 22 Apr 2022 00:29:20 -0000 1.9
> +++ kstat.c 29 Apr 2022 01:43:31 -0000
> @@ -15,6 +15,8 @@
> */
>
> #include <ctype.h>
> +#include <limits.h>
> +#include <signal.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <stddef.h>
> @@ -104,6 +106,7 @@ kstat_cmp(const struct kstat_entry *ea,
> RBT_PROTOTYPE(kstat_tree, kstat_entry, entry, kstat_cmp);
> RBT_GENERATE(kstat_tree, kstat_entry, entry, kstat_cmp);
>
> +static void handle_alrm(int);
> static struct kstat_filter *
> kstat_filter_parse(char *);
> static int kstat_filter_entry(struct kstat_filters *,
> @@ -134,16 +137,17 @@ main(int argc, char *argv[])
> int fd;
> const char *errstr;
> int ch;
> - struct timespec interval = { 0, 0 };
> + struct itimerval itv;
> + unsigned int wait = 0;
> + sigset_t empty;
> int i;
>
> while ((ch = getopt(argc, argv, "w:")) != -1) {
> switch (ch) {
> case 'w':
> - interval.tv_sec = strtonum(optarg, 1, 100000000,
> - &errstr);
> + wait = strtonum(optarg, 1, UINT_MAX, &errstr);
> if (errstr != NULL)
> - errx(1, "wait %s: %s", optarg, errstr);
> + errx(1, "wait is %s: %s", errstr, optarg);
> break;
> default:
> usage();
> @@ -165,15 +169,21 @@ main(int argc, char *argv[])
> if (ioctl(fd, KSTATIOC_VERSION, &version) == -1)
> err(1, "kstat version");
>
> - kstat_list(&kt, fd, version, &kfs);
> - kstat_print(&kt);
> -
> - if (interval.tv_sec == 0)
> + if (wait == 0) {
> + kstat_list(&kt, fd, version, &kfs);
> + kstat_print(&kt);
> return (0);
> + }
>
> + sigemptyset(&empty);
> + signal(SIGALRM, handle_alrm);
> + itv.it_value.tv_sec = wait;
> + itv.it_value.tv_usec = 0;
> + itv.it_interval = itv.it_value;
> + if (setitimer(ITIMER_REAL, &itv, NULL) == -1)
> + err(1, "setitimer");
> for (;;) {
> - nanosleep(&interval, NULL);
> -
> + sigsuspend(&empty);
> kstat_read(&kt, fd);
> kstat_print(&kt);
> }
> @@ -547,4 +557,9 @@ kstat_read(struct kstat_tree *kt, int fd
> if (ioctl(fd, KSTATIOC_FIND_ID, ksreq) == -1)
> err(1, "update id %llu", ksreq->ks_id);
> }
> +}
> +
> +static void
> +handle_alrm(int signo)
> +{
> }
