Date: Tue, 20 Nov 2001 09:30:01 -0800 (PST) From: "Andrew R. Reiter" <arr@FreeBSD.org> To: freebsd-bugs@FreeBSD.org Subject: Re: bin/32138: better progress reporting for dump(8) Message-ID: <200111201730.fAKHU1X67179@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: "Andrew R. Reiter" <arr@FreeBSD.org>
To: Mikhail Teterin <mi@aldan.algebra.com>
Cc: FreeBSD-gnats-submit@FreeBSD.org
Subject: Re: bin/32138: better progress reporting for dump(8)
Date: Tue, 20 Nov 2001 12:26:28 -0500 (EST)
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?
Cheers,
Andrew
On Tue, 20 Nov 2001, Mikhail Teterin wrote:
:
:>Number: 32138
:>Category: bin
:>Synopsis: better progress reporting for dump(8)
:>Confidential: no
:>Severity: non-critical
:>Priority: low
:>Responsible: freebsd-bugs
:>State: open
:>Quarter:
:>Keywords:
:>Date-Required:
:>Class: change-request
:>Submitter-Id: current-users
:>Arrival-Date: Tue Nov 20 09:00:00 PST 2001
:>Closed-Date:
:>Last-Modified:
:>Originator: Mikhail Teterin
:>Release: FreeBSD 5.0-CURRENT i386
:>Organization:
:Virtual Estates, Inc.
:>Environment:
:System: FreeBSD aldan.algebra.com 5.0-CURRENT FreeBSD 5.0-CURRENT #6: Mon Nov 5 08:47:03 EST 2001 mi@aldan.algebra.com:/ccd/obj/ccd/src/sys/DEBUG i386
:
:
:>Description:
:
: The included patch makes dump(8) update its proctitle to reflect
: what it is doing at the moment (including the percentage of the
: task completed). It also provides reasonable handling of SIGINFO,
: and removes the arbitrary (and confusing) requirement, that at
: least 500 blocks must be written before the next progress report
: line is output.
:
: The first version of the patch was critisized on -arch for doing
: things a signal handler is not allowed to do. This version simply
: alters the scheduled time of the next report to ensure it happens
: after the next block is being written.
:
:>How-To-Repeat:
:
:>Fix:
:
: Please, commit this, or agree to become my mentor for the src
: tree -- I'll be glad to commit this myself :-)
:
:cvs server: Diffing .
: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 16:49:14 -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 16:49:14 -0000
:@@ -101,6 +101,7 @@
: int query __P((char *question));
: void quit __P((const char *fmt, ...)) __printflike(1, 2);
:-void timeest __P((void));
:+void timeest __P((int)); /* accepts the signal number */
: 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 16:49:14 -0000
:@@ -52,4 +52,5 @@
: #include <string.h>
: #include <stdarg.h>
:+#include <signal.h>
: #include <unistd.h>
: #include <utmp.h>
:@@ -191,14 +192,26 @@
:
: void
:-timeest()
:+timeest(signal)
: {
: time_t tnow;
: int deltat;
:
:- (void) time((time_t *) &tnow);
:+ if (signal != 0) {
:+ /*
:+ * We might be called to handle SIGINFO. Re-schedule
:+ * the reporting to occur the next time we are called
:+ * regularly and bail out -- don't do reporting as a
:+ * signal handler -- it involves malloc-ing and other
:+ * things signal handler are not supposed to do.
:+ */
:+ /*
:+ * 300 seconds is a constant only used in this function
:+ */
:+ tschedule -= 300;
:+ return;
:+ }
:+ (void) time(&tnow);
: if (tnow >= tschedule) {
: tschedule = tnow + 300;
:- if (blockswritten < 500)
:- return;
: deltat = tstart_writing - tnow +
: (1.0 * (tnow - tstart_writing))
:@@ -207,4 +220,5 @@
: (blockswritten * 100.0) / tapesize,
: deltat / 3600, (deltat % 3600) / 60);
:+ title();
: }
: }
:@@ -235,4 +249,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 16:49:15 -0000
:@@ -309,5 +309,5 @@
: startnewtape(0);
: }
:- timeest();
:+ timeest(0);
: }
:
:@@ -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, timeest); /* report progress on SIGINFO */
:
: asize = 0;
:>Release-Note:
:>Audit-Trail:
:>Unformatted:
:
:To Unsubscribe: send mail to majordomo@FreeBSD.org
:with "unsubscribe freebsd-bugs" in the body of the message
:
--
Andrew R. Reiter
arr@watson.org
arr@FreeBSD.org
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?200111201730.fAKHU1X67179>