Re: proctitle progress reporting for dump(8)
On Wed, 5 Sep 2001, Mikhail Teterin wrote: On 5 Sep, Bruce Evans wrote: snprintf, strlen, vsnprintf, sysctl, sysctlbyname I think all of these are safe in practice. It also accesses some variables that are not safe to access in a signal handler (non-auto ones that are not of type volatile sig_atomic_t or are accessed by reads). This is unsafe in practice. E.g., concurrent calls to setproctitle() might corrupt the state of the ps_strings variable. Mmm, I don't know what those strings are used for -- what's the worst, that could happen here -- corrupted PS output, or worse? Probably security holes from buffer overruns. strlen() on the static buffer may give a result larger than the buffer size if it is called concurrently with an snprintf() that is modifying the buffer. Then vsnprintf(buf + len, sizeof(buf) - len, fmt, ap) gives a buffer overrun. In any case, it seems, like a simple lock -- a static variable in the signal handler would work: static signal_handling; ... if (signal_handling) return; if (signal) signal_handling = 1; ... signal_handling = 0; Or is this not safe enough -- race of both signal handlers checking for the signal_handling at the same time? What would be the right way to do this generally? Thanks. The signal mask would normally prevent concurrent calls from the SIGINFO handler, but in general you need more than the above (an atomic test-and- set of `signal_handling'). setproctitle() is a library function so it should do this generally or not at all. But it can't do this, since it has no way to handle the `signal_handling' case. It can't just return, because its spec doesn't permit it to fail, and it can't give up control in non-threaded programs. Bruce To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: proctitle progress reporting for dump(8)
On Wed, 5 Sep 2001, Terry Lambert wrote: Mikhail Teterin wrote: Also, printf() allocates memory for floating point, so if that percentage is a floating point calculation, then you are in double trouble, since you are not allowed to call malloc() in a signal handler. That's interesting... I can modify it a bit, to round the percentage to fit the %d if called as a signal handler. Thanks. Anything else? If setproctitle() calls malloc/strsave/etc., it is not safe to call in a signal handler. Neither is setproctitle() (since it is not in the list of functions that are safe to call in a signal handler), so this is moot. I'm not saying it does, I'm saying I haven't looked at the code in libc for the function, and you should, before using it in a signal handler... setproctitle() directly calls the following functions that are not safe to call in a signal handler (since they are not in the magic list). snprintf, strlen, vsnprintf, sysctl, sysctlbyname I think all of these are safe in practice. It also accesses some variables that are not safe to access in a signal handler (non-auto ones that are not of type volatile sig_atomic_t or are accessed by reads). This is unsafe in practice. E.g., concurrent calls to setproctitle() might corrupt the state of the ps_strings variable. Bruce To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: proctitle progress reporting for dump(8)
On 5 Sep, Bruce Evans wrote: snprintf, strlen, vsnprintf, sysctl, sysctlbyname I think all of these are safe in practice. It also accesses some variables that are not safe to access in a signal handler (non-auto ones that are not of type volatile sig_atomic_t or are accessed by reads). This is unsafe in practice. E.g., concurrent calls to setproctitle() might corrupt the state of the ps_strings variable. Mmm, I don't know what those strings are used for -- what's the worst, that could happen here -- corrupted PS output, or worse? In any case, it seems, like a simple lock -- a static variable in the signal handler would work: static signal_handling; ... if (signal_handling) return; if (signal) signal_handling = 1; ... signal_handling = 0; Or is this not safe enough -- race of both signal handlers checking for the signal_handling at the same time? What would be the right way to do this generally? Thanks. In this particular case, timeest is called fairly often, but simply returns if not enough time elapsed since last report. I can make the signal handler set the flag, which will make timeest report things the next time it is called, even if 5 minutes did not elapse. This way, signal handler will only deal with a variable assignment -- all of the reporting (including proctitle update) will happen shortly afterwards -- on a normal stack. -mi To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: proctitle progress reporting for dump(8)
Mikhail Teterin wrote: Also, printf() allocates memory for floating point, so if that percentage is a floating point calculation, then you are in double trouble, since you are not allowed to call malloc() in a signal handler. That's interesting... I can modify it a bit, to round the percentage to fit the %d if called as a signal handler. Thanks. Anything else? If setproctitle() calls malloc/strsave/etc., it is not safe to call in a signal handler. I'm not saying it does, I'm saying I haven't looked at the code in libc for the function, and you should, before using it in a signal handler... -- Terry To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: proctitle progress reporting for dump(8)
Mikhail Teterin wrote: On 1 Sep, Jeroen Ruigrok/Asmodai wrote: -On [20010901 19:00], Mikhail Teterin ([EMAIL PROTECTED]) wrote: 79240 ?? S 0:06,85 dump: /dev/da0h(0): 92.44% done, finished in 0:43 (dump) Looks nice. Would definately be an improvement. I would like it. How often does it update the proctitle? Whenever it outputs a line to the stderr -- I personally find no regularity in that :(. SIGINFO handling is a different thing, though. I'll look at that too. Thanks, It would be nice to have in a SIGINFO handler as well, but since your application is dump run by Amanda as a background process, a SIGINFO is not very useful, unless it was being suggested that the setproctitle() be called from a SIGINFO handler? I'm pretty sure that this would not be a valid thing to do; SIGINFO itself is also problematic, since if you redirect output to a file, etc., you'd really need to call fflush() in the signal handler, which is not supposed to be done, as it is not one of the permitted calls. Also, printf() allocates memory for floating point, so if that percentage is a floating point calculation, then you are in double trouble, since you are not allowed to call malloc() in a signal handler. -- Terry To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: proctitle progress reporting for dump(8)
On Sun, Sep 02, 2001 at 04:15:14PM -0400, Mikhail Teterin wrote: [...] The SIGINFO handling seemed to be as simple as: --- main.c2001/07/09 03:06:56 1.26 +++ main.c2001/09/02 19:58:21 @@ -274,2 +274,4 @@ + if (signal(SIGINFO, SIG_IGN) != SIG_IGN) + signal(SIGHUP, sig); ^^ that should be SIGINFO. Cheers, -- Ruslan Ermilov Oracle Developer/DBA, [EMAIL PROTECTED] Sunbay Software AG, [EMAIL PROTECTED] FreeBSD committer, +380.652.512.251Simferopol, Ukraine http://www.FreeBSD.org The Power To Serve http://www.oracle.com Enabling The Information Age To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: proctitle progress reporting for dump(8)
On Sat, Sep 01, 2001 at 10:36:45PM -0400, Garrett Wollman wrote: On Sun, 02 Sep 2001 00:39:22 +0200, Arne Dag Fidjestøl [EMAIL PROTECTED] said: Could you please clarify your position on this issue? Is setproctitle() the wrong way to do this, and if so, why? I don't expect setproctitle() to be useful to me one way or the other. SIGINFO, on the other hand, would help keep me from going out of my mind while sitting at the single-user console waiting for some filesystem move to finish. More regular updates would be almost as good. And you get an answer whenever you ask the question. Yes, please do it dd(1) way. Cheers, -- Ruslan Ermilov Oracle Developer/DBA, [EMAIL PROTECTED] Sunbay Software AG, [EMAIL PROTECTED] FreeBSD committer, +380.652.512.251Simferopol, Ukraine http://www.FreeBSD.org The Power To Serve http://www.oracle.com Enabling The Information Age To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: proctitle progress reporting for dump(8)
On 3 Sep, Terry Lambert wrote: I would like it. How often does it update the proctitle? Whenever it outputs a line to the stderr -- I personally find no regularity in that :(. SIGINFO handling is a different thing, though. I'll look at that too. Thanks, It would be nice to have in a SIGINFO handler as well, but since your application is dump run by Amanda as a background process, a SIGINFO is not very useful, unless it was being suggested that the setproctitle() be called from a SIGINFO handler? Yes that's what is being suggested. The output line will be the same, so no dump-output-parsing scripts should break (including Amanda's). Please, read the patches. I'm pretty sure that this would not be a valid thing to do; Mmm, why? SIGINFO itself is also problematic, since if you redirect output to a file, etc., you'd really need to call fflush() Why? The line will eventually be saved without an explicit fflush(), but the proctitle will be set immediately... Besides, in case of dump, it is the stderr, that has no buffering, AFAIK :) Also, printf() allocates memory for floating point, so if that percentage is a floating point calculation, then you are in double trouble, since you are not allowed to call malloc() in a signal handler. That's interesting... I can modify it a bit, to round the percentage to fit the %d if called as a signal handler. Thanks. Anything else? -mi To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: proctitle progress reporting for dump(8)
Ok, attached is the patch addding a function, which sets the proctitle
to the last output message and several calls to this function in places,
where it looked useful to me. May be, I added too many, and/or skipped
some...
Note, that I intentially did not put this functionality into the msg()
function itself -- not all messages need to be placed into the title.
But a call to my new title(void) function can be placed whereever deemed
useful. Only those, that can be followed by a long wait...
The only tricks here are to replace the \n with the \0 in the lastmsg
and to restore the title to the original before forking.
The SIGINFO handling seemed to be as simple as:
--- main.c 2001/07/09 03:06:56 1.26
+++ main.c 2001/09/02 19:58:21
@@ -274,2 +274,4 @@
+ if (signal(SIGINFO, SIG_IGN) != SIG_IGN)
+ signal(SIGHUP, sig);
if (signal(SIGHUP, SIG_IGN) != SIG_IGN)
@@ -527,2 +536,5 @@
switch(signo) {
+ case SIGINFO:
+ timeest();
+ break;
case SIGALRM:
But it just does not work :( I tried Ctrl-T and I tried killall -- no
output, besides the usual:
load: 0.11 cmd: dump 69089 [running] 0.00u 0.00s 0% 392k
Any suggestions? Thanks!
I'm only a ports committer, so if the proctitle patch is found
acceptable (wow!) -- could someone please commit it? Or tell me to
send-pr it...
-mi
Index: dump.h
===
RCS file: /home/ncvs/src/sbin/dump/dump.h,v
retrieving revision 1.9
diff -U1 -r1.9 dump.h
--- dump.h 2001/08/10 23:12:10 1.9
+++ dump.h 2001/09/02 19:58:20
@@ -104,2 +104,3 @@
void timeest __P((void));
+void title __P((void));
time_t unctime __P((char *str));
Index: main.c
===
RCS file: /home/ncvs/src/sbin/dump/main.c,v
retrieving revision 1.26
diff -U1 -r1.26 main.c
--- main.c 2001/07/09 03:06:56 1.26
+++ main.c 2001/09/02 19:58:21
@@ -332,2 +334,3 @@
}
+ title();
sync();
@@ -358,2 +361,3 @@
msg(mapping (Pass I) [regular files]\n);
+ title();
anydirskipped = mapfiles(maxino, tapesize);
@@ -361,2 +365,3 @@
msg(mapping (Pass II) [directories]\n);
+ title();
while (anydirskipped) {
@@ -410,2 +415,3 @@
}
+ title();
@@ -423,2 +429,3 @@
msg(dumping (Pass III) [directories]\n);
+ title();
dirty = 0; /* XXX just to get gcc to shut up */
@@ -441,2 +448,3 @@
msg(dumping (Pass IV) [regular files]\n);
+ title();
for (map = dumpinomap, ino = 1; ino maxino; ino++) {
@@ -478,2 +486,3 @@
spcl.c_tapea / (tend_writing - tstart_writing));
+ title();
@@ -536,2 +548,3 @@
msg(Rewriting attempted as response to unknown signal.\n);
+ title();
(void)fflush(stderr);
Index: optr.c
===
RCS file: /home/ncvs/src/sbin/dump/optr.c,v
retrieving revision 1.12
diff -U1 -r1.12 optr.c
--- optr.c 2001/01/29 09:45:51 1.12
+++ optr.c 2001/09/02 19:58:21
@@ -203,2 +203,3 @@
+ setproctitle(NULL); /* restore the proctitle modified by title() */
switch (pid = fork()) {
@@ -305,2 +306,3 @@
deltat / 3600, (deltat % 3600) / 60);
+ title();
}
@@ -333,2 +335,28 @@
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
+* 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.13
diff -U1 -r1.13 tape.c
--- tape.c 2001/01/28 21:21:37 1.13
+++ tape.c 2001/09/02 19:58:22
@@ -533,2 +533,3 @@
*/
+ setproctitle(NULL); /* restore the proctitle modified by title() */
childpid = fork();
Re: proctitle progress reporting for dump(8)
On Sat, 1 Sep 2001 21:55:09 +0200, Jeroen Ruigrok/Asmodai [EMAIL PROTECTED] said: You mean dump should get a signal handler for SIGINFO to print/display the current status of the application? Yes! Just like in fsck, and for the same reasons. -GAWollman To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: proctitle progress reporting for dump(8)
On Sat, 1 Sep 2001 19:47:06 +0200, Jeroen Ruigrok/Asmodai [EMAIL PROTECTED] said: 79240 ?? S 0:06,85 dump: /dev/da0h(0): 92.44% done, finished in 0:43 (dump) SIGINFO! SIGINFO! SIGINFO! You'd still need somewhere to put the status message; the dump process above has no controlling terminal. -adf To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: proctitle progress reporting for dump(8)
On Sat, 01 Sep 2001 22:48:37 +0200, Arne Dag Fidjestøl [EMAIL PROTECTED] said: You'd still need somewhere to put the status message; the dump process above has no controlling terminal. If it has no controlling terminal then it's not going to receive ctty signals like SIGINFO. -GAWollman To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: proctitle progress reporting for dump(8)
If it has no controlling terminal then it's not going to receive ctty signals like SIGINFO. Unless you send the signal manually. But I agree, SIGINFO is not a good solution here :) -adf To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: proctitle progress reporting for dump(8)
-On [20010901 23:24], Arne Dag Fidjestøl ([EMAIL PROTECTED]) wrote: You'd still need somewhere to put the status message; the dump process above has no controlling terminal. Putting it into syslog might be a bit too verbose for this? -- Jeroen Ruigrok van der Werven/Asmodai asmodai@[wxs.nl|freebsd.org|xmach.org] Documentation nutter/C-rated Coder, finger [EMAIL PROTECTED] http://www.freebsd.org/doc/en_US.ISO8859-1/books/developers-handbook/ Give me the rest to accept what I cannot change, give me the strength to change when I can, and give me the wisdom to see the difference... To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: proctitle progress reporting for dump(8)
On Sat, 01 Sep 2001 23:08:48 +0200, Arne Dag Fidjestøl [EMAIL PROTECTED] said: But I agree, SIGINFO is not a good solution here :) I'm not sure who you're agreeing with, since I did not say that. -GAWollman To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: proctitle progress reporting for dump(8)
I like it. I se no problem. Does this look like a good idea to anyone else? 79239 ?? I 0:00,89 dump 0ushf 1048576 0 - /dev/da0h (dump) To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: proctitle progress reporting for dump(8)
On Sat, 01 Sep 2001 23:08:48 +0200, Arne Dag Fidjestøl [EMAIL PROTECTED] said: But I agree, SIGINFO is not a good solution here :) I'm not sure who you're agreeing with, since I did not say that. I apologize for the remark, however tongue-in-cheek it was intended. Could you please clarify your position on this issue? Is setproctitle() the wrong way to do this, and if so, why? -adf To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: proctitle progress reporting for dump(8)
On 1 Sep, Jeroen Ruigrok/Asmodai wrote: -On [20010901 19:00], Mikhail Teterin ([EMAIL PROTECTED]) wrote: 79240 ?? S 0:06,85 dump: /dev/da0h(0): 92.44% done, finished in 0:43 (dump) Looks nice. Would definately be an improvement. I would like it. How often does it update the proctitle? Whenever it outputs a line to the stderr -- I personally find no regularity in that :(. SIGINFO handling is a different thing, though. I'll look at that too. Thanks, -mi To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: proctitle progress reporting for dump(8)
On Sun, 02 Sep 2001 00:39:22 +0200, Arne Dag Fidjestøl [EMAIL PROTECTED] said: Could you please clarify your position on this issue? Is setproctitle() the wrong way to do this, and if so, why? I don't expect setproctitle() to be useful to me one way or the other. SIGINFO, on the other hand, would help keep me from going out of my mind while sitting at the single-user console waiting for some filesystem move to finish. More regular updates would be almost as good. -GAWollman To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: proctitle progress reporting for dump(8)
-On [20010901 19:00], Mikhail Teterin ([EMAIL PROTECTED]) wrote: 79240 ?? S 0:06,85 dump: /dev/da0h(0): 92.44% done, finished in 0:43 (dump) Does anyone think, it is a bad idea? If no, I'll send-pr the patch... For me, dump is driven by a remote amanda and its nice to know, when it is going to be over (I have a fairly slow link to the backup server). Looks nice. Would definately be an improvement. I would like it. How often does it update the proctitle? -- Jeroen Ruigrok van der Werven/Asmodai asmodai@[wxs.nl|freebsd.org|xmach.org] Documentation nutter/C-rated Coder, finger [EMAIL PROTECTED] http://www.freebsd.org/doc/en_US.ISO8859-1/books/developers-handbook/ I dream of gardens in the desert sand... To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: proctitle progress reporting for dump(8)
On Sat, Sep 01, 2001 at 07:45:17PM +0200, Leif Neland wrote: I like it. I se no problem. Does this look like a good idea to anyone else? 79239 ?? I 0:00,89 dump 0ushf 1048576 0 - /dev/da0h (dump) Nice idea IMO. -- | / o / / _ Arnhem, The Netherlands email: [EMAIL PROTECTED] |/|/ / / /( (_) Bulte To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: proctitle progress reporting for dump(8)
On Sat, 1 Sep 2001 19:47:06 +0200, Jeroen Ruigrok/Asmodai [EMAIL PROTECTED] said: 79240 ?? S 0:06,85 dump: /dev/da0h(0): 92.44% done, finished in 0:43 (dump) SIGINFO! SIGINFO! SIGINFO! -GAWollman To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: proctitle progress reporting for dump(8)
In message [EMAIL PROTECTED], Garrett Wollman w rites: On Sat, 1 Sep 2001 19:47:06 +0200, Jeroen Ruigrok/Asmodai [EMAIL PROTECTED] said: 79240 ?? S 0:06,85 dump: /dev/da0h(0): 92.44% done, finished in 0:4 3 (dump) SIGINFO! SIGINFO! SIGINFO! Much better idea! Regards, Phone: (250)387-8437 Cy SchubertFax: (250)387-5766 Team Leader, Sun/Alpha Team Internet: [EMAIL PROTECTED] Open Systems Group, ITSD Ministry of Management Services Province of BC To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
Re: proctitle progress reporting for dump(8)
-On [20010901 21:48], Garrett Wollman ([EMAIL PROTECTED]) wrote: On Sat, 1 Sep 2001 19:47:06 +0200, Jeroen Ruigrok/Asmodai [EMAIL PROTECTED] said: 79240 ?? S 0:06,85 dump: /dev/da0h(0): 92.44% done, finished in 0:43 (dump) SIGINFO! SIGINFO! SIGINFO! Heh. :) Let me elaborate your, erm, somewhat terse reply so that I understand you. You mean dump should get a signal handler for SIGINFO to print/display the current status of the application? -- Jeroen Ruigrok van der Werven/Asmodai asmodai@[wxs.nl|freebsd.org|xmach.org] Documentation nutter/C-rated Coder, finger [EMAIL PROTECTED] http://www.freebsd.org/doc/en_US.ISO8859-1/books/developers-handbook/ Someone help me, I think I've lost control... To Unsubscribe: send mail to [EMAIL PROTECTED] with unsubscribe freebsd-current in the body of the message
