Date: Tue, 20 Nov 2001 11:10:04 -0800 (PST) From: Mikhail Teterin <mi@aldan.algebra.com> To: freebsd-bugs@FreeBSD.org Subject: Re: bin/32138: better progress reporting for dump(8) Message-ID: <200111201910.fAKJA4v87817@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR bin/32138; it has been noted by GNATS.
From: Mikhail Teterin <mi@aldan.algebra.com>
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 <string.h>
#include <stdarg.h>
+#include <signal.h>
#include <unistd.h>
#include <utmp.h>
@@ -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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200111201910.fAKJA4v87817>