From owner-freebsd-bugs Tue Nov 20 11:10:23 2001 Delivered-To: freebsd-bugs@hub.freebsd.org Received: from freefall.freebsd.org (freefall.FreeBSD.org [216.136.204.21]) by hub.freebsd.org (Postfix) with ESMTP id 1AAD137B417 for ; Tue, 20 Nov 2001 11:10:04 -0800 (PST) Received: (from gnats@localhost) by freefall.freebsd.org (8.11.4/8.11.4) id fAKJA4v87817; Tue, 20 Nov 2001 11:10:04 -0800 (PST) (envelope-from gnats) Date: Tue, 20 Nov 2001 11:10:04 -0800 (PST) Message-Id: <200111201910.fAKJA4v87817@freefall.freebsd.org> To: freebsd-bugs@FreeBSD.org Cc: From: Mikhail Teterin Subject: Re: bin/32138: better progress reporting for dump(8) Reply-To: Mikhail Teterin Sender: owner-freebsd-bugs@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.org The following reply was made to PR bin/32138; it has been noted by GNATS. From: Mikhail Teterin To: iedowse@maths.tcd.ie, arr@FreeBSD.org Cc: FreeBSD-gnats-submit@FreeBSD.org Subject: Re: bin/32138: better progress reporting for dump(8) Date: Tue, 20 Nov 2001 14:07:34 -0500 (EST) On 20 Nov, Ian Dowse wrote: > In message <200111201651.fAKGp7s38773@aldan.algebra.com>, Mikhail Teterin write > s: >>-void timeest __P((void)); >>+void timeest __P((int)); /* accepts the signal number */ > >>+ signal(SIGINFO, timeest); /* report progress on SIGINFO */ > I think it would be cleaner to add a simple signal handler which sets > a flag, and then check this where necessary. Calling a signal handler > function directly is weird, and using a magic value for the signal > number is evil :-) Yes, a weird evil :) Ok, I'll split it -- I did not want to create another global function, to be honest. > What happens if multiple ^Ts are sent and tschedule gets decremented a > few times in a row? The check is only for tnow >= tschedule. So, no problem, unless tschedule gets decremented into the positive territory (imagine the number of ^T this would require), in which case, the progress reporting will cease. Big deal... The version I last presented to -arch set tschedule to the current time, but that required a call to time(), which is not a problem, really, but is not needed either. On 20 Nov, Andrew R. Reiter wrote: > The signal handler in the original code was not signal safe and > neither is the code givenhere.. I'd like to see the code be fixed from > this problem, willing to make it signal safe? What's not safe about it? The only thing happening in the signal handler is ``tschedule -= 300;''. I think, it is quite safe -- see above I can even make it ``if (tschedule > 300) tschedule -= 300;'' :-) Could you be more specific? Thanks! -mi How about this: Index: dump.8 =================================================================== RCS file: /home/ncvs/src/sbin/dump/dump.8,v retrieving revision 1.39 diff -U2 -r1.39 dump.8 --- dump.8 15 Jul 2001 14:00:19 -0000 1.39 +++ dump.8 20 Nov 2001 19:07:30 -0000 @@ -320,5 +320,6 @@ .Pp .Nm Dump -tells the operator what is going on at periodic intervals, +tells the operator what is going on at periodic intervals -- +every 5 minutes, or promptly after receiving SIGINFO -- including usually low estimates of the number of blocks to write, the number of tapes it will take, the time to completion, and Index: dump.h =================================================================== RCS file: /home/ncvs/src/sbin/dump/dump.h,v retrieving revision 1.11 diff -U2 -r1.11 dump.h --- dump.h 17 Nov 2001 00:06:55 -0000 1.11 +++ dump.h 20 Nov 2001 19:07:30 -0000 @@ -96,4 +96,5 @@ /* operator interface functions */ void broadcast __P((char *message)); +void infosch __P((int)); void lastdump __P((int arg)); /* int should be char */ void msg __P((const char *fmt, ...)) __printflike(1, 2); @@ -103,4 +104,5 @@ void timeest __P((void)); time_t unctime __P((char *str)); +void title __P((void)); /* mapping rouintes */ Index: optr.c =================================================================== RCS file: /home/ncvs/src/sbin/dump/optr.c,v retrieving revision 1.16 diff -U2 -r1.16 optr.c --- optr.c 17 Nov 2001 00:06:55 -0000 1.16 +++ optr.c 20 Nov 2001 19:07:30 -0000 @@ -52,4 +52,5 @@ #include #include +#include #include #include @@ -188,5 +189,5 @@ */ -time_t tschedule = 0; +static time_t tschedule = 0; void @@ -196,9 +197,7 @@ int deltat; - (void) time((time_t *) &tnow); + (void) time(&tnow); if (tnow >= tschedule) { tschedule = tnow + 300; - if (blockswritten < 500) - return; deltat = tstart_writing - tnow + (1.0 * (tnow - tstart_writing)) @@ -207,7 +206,21 @@ (blockswritten * 100.0) / tapesize, deltat / 3600, (deltat % 3600) / 60); + title(); } } +/* + * Reschedule the next printout of the estimate + */ +void +infosch(int signal) { + /* + * 300 seconds -- 5 minutes -- is the magical constant, + * only used in this file + */ + if (tschedule > 300) + tschedule -= 300; +} + void #if __STDC__ @@ -235,4 +248,31 @@ (void) vsnprintf(lastmsg, sizeof(lastmsg), fmt, ap); va_end(ap); +} + +/* + * This function can be called to place, what msg() above pushed to + * stderr, into the process title, viewable with the ps-command. + * A side effect of this function, is it replaces the final '\n' (if any) + * with the '\0' in the global variable lastmsg -- to avoid the literal + * "\n" being put into the proctitle. + * So, if the lastmsg needs to be output elsewhere, that should happen + * before calling title(). + */ +void title() +{ + int lastlen; + + lastlen = strlen(lastmsg); + if (lastmsg[lastlen-1] == '\n') + lastmsg[lastlen-1] = '\0'; + + /* + * It would be unwise to run multiple dumps of same disk + * at the same time. So ``disk'' is sufficient for + * identifying, to which family of dump processes this + * one belongs -- the other processes continue to have + * the original titles. + */ + setproctitle("%s: %s", disk, lastmsg); } Index: tape.c =================================================================== RCS file: /home/ncvs/src/sbin/dump/tape.c,v retrieving revision 1.14 diff -U2 -r1.14 tape.c --- tape.c 17 Nov 2001 00:06:55 -0000 1.14 +++ tape.c 20 Nov 2001 19:07:31 -0000 @@ -522,4 +522,5 @@ * All signals are inherited... */ + setproctitle(NULL); /* restore the proctitle modified by title() */ childpid = fork(); if (childpid < 0) { @@ -612,4 +613,5 @@ enslave(); /* Share open tape file descriptor with slaves */ + signal(SIGINFO, infosch); /* report progress on SIGINFO */ asize = 0; To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-bugs" in the body of the message