[Cluster-devel] [PATCH v2] glocktop: Fix a tight loop under nohup

[Andreas Gruenbacher]

On Tue, Mar 1, 2016 at 8:43 PM, Andrew Price <anprice at redhat.com> wrote:
> When glocktop is run under nohup, /dev/null is redirected to stdin,
> which flags up EOF and causes the file descriptor to always be in a
> ready state when select()ed. This causes select() to return immediately
> instead of timing out after the refresh delay.
>
> Remove stdin from the fd_set when we get EOF to avoid getting into a
> tight loop when run under nohup.
>
> Also fix up some messages relating to the refresh interval.
>
> Signed-off-by: Andrew Price <anprice at redhat.com>
> ---
>  gfs2/glocktop/glocktop.c | 24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/gfs2/glocktop/glocktop.c b/gfs2/glocktop/glocktop.c
> index 43c3c3f..01b8099 100644
> --- a/gfs2/glocktop/glocktop.c
> +++ b/gfs2/glocktop/glocktop.c
> @@ -147,7 +147,6 @@ enum summary_types {
>  char debugfs[PATH_MAX];
>  int termcols = 80, termlines = 30, done = 0;
>  unsigned glocks = 0;
> -int refresh_time = REFRESH_TIME;
>  const char *termtype;
>  WINDOW *wind;
>  int bufsize = 4 * 1024 * 1024;
> @@ -1625,7 +1624,7 @@ static void usage(void)
>                "[-c] [-D] [-H] [-r] [-t]\n");
>         printf("\n");
>         printf("-i : Runs glocktop in interactive mode.\n");
> -       printf("-d : delay between refreshes, in seconds (default: 3).\n");
> +       printf("-d : delay between refreshes, in seconds (default: %d).\n", REFRESH_TIME);
>         printf("-n : stop after <iter> refreshes.\n");
>         printf("-H : don't show Held glocks, even if not waited on, excluding "
>                "iopen\n");
> @@ -1645,7 +1644,7 @@ int main(int argc, char **argv)
>         char fn[PATH_MAX];
>         struct dirent *dent;
>         int retval;
> -       struct timeval tv;
> +       int refresh_time = REFRESH_TIME;
>         fd_set readfds;
>         char string[96];
>         int ch, i, dlmwaiters = 0, dlmgrants = 0;
> @@ -1654,6 +1653,7 @@ int main(int argc, char **argv)
>         int show_held = 1, help = 0;
>         int interactive = 0;
>         int summary = 10;
> +       int nfds;
>
>         prog_name = argv[0];
>         memset(glock, 0, sizeof(glock));
> @@ -1669,7 +1669,7 @@ int main(int argc, char **argv)
>                         refresh_time = atoi(optarg);
>                         if (refresh_time < 1) {
>                                 fprintf(stderr, "Error: delay %d too small; "
> -                                       "must be > 1\n", refresh_time);
> +                                       "must be at least 1\n", refresh_time);
>                                 exit(-1);
>                         }
>                         break;
> @@ -1758,7 +1758,13 @@ int main(int argc, char **argv)
>                 bufsize /= 2;
>         }
>
> +       FD_ZERO(&readfds);
> +       FD_SET(STDIN_FILENO, &readfds);
> +       nfds = STDIN_FILENO + 1;
> +
>         while (!done) {
> +               struct timeval tv;
> +
>                 sprintf(fn, "%s/gfs2/", debugfs);
>                 dir = opendir(fn);
>
> @@ -1828,9 +1834,7 @@ int main(int argc, char **argv)
>                 closedir(dir);
>                 tv.tv_sec = refresh_time;
>                 tv.tv_usec = 0;
> -               FD_ZERO(&readfds);
> -               FD_SET(0, &readfds);
> -               retval = select(1, &readfds, NULL, NULL, &tv);
> +               retval = select(nfds, &readfds, NULL, NULL, &tv);

No, the readfds set must be initialized in the loop, before each call
to select. It is modified in-place.

>                 if (retval) {
>                         if (interactive)
>                                 ch = getch();
> @@ -1856,6 +1860,12 @@ int main(int argc, char **argv)
>                                 if (refresh_time < 1)
>                                         refresh_time = 1;
>                                 break;
> +                       /* When we get EOF on stdin, remove it from the fd_set
> +                          to avoid shorting out the select() */
> +                       case EOF:
> +                               FD_CLR(STDIN_FILENO, &readfds);
> +                               nfds = 0;
> +                               break;
>                         }
>                 }
>                 iters_done++;
> --
> 2.4.3
>

Thanks,
Andreas