On 08/04/2011 02:53 AM, Christoph Hellwig wrote:
On Wed, Aug 03, 2011 at 02:28:54PM -0500, Bill Kendall wrote:
Here's some background explaining why things are done as they
are now, from my understanding of the code.
The regular handler won't acquire a lock. The signal handler is
replaced because the rules are different when receiving a signal
while in a dialog. For instance, SIGINT normally means interrupt
the dump session, but in a dialog we just return a caller-supplied
value indicating the interrupt.
When a dialog is required, the caller does this:
dlog_begin(); // grabs mlog_lock
dlog_*_query(); // ends up in promptinput()
dlog_end(); // releases mlog_lock
I think the purpose of holding the lock is simply to prevent
other output on the terminal while waiting for a response.
Ok, that makes some sense.
Any thread may issue a dialog, and it's possible that while
a thread is sitting in a dialog, the main thread may try to
log a message (e.g., progress report) and get blocked on the
mlog lock. At this point nobody would be able to handle signals --
the main thread blocks all signals except while in sigsuspend,
and other threads always block signals. So we unblock the
signals in the current thread to ensure some thread is available
to handle them.
Unblocking the signals during the dialog, but still using the normal
signal handler for it would solve that problem, right?
Right, with some rework of that handler. It would have to do
something like:
case SIGINT:
if (is_dialog_active(SIGINT))
dlg_sigterm_received = BOOL_TRUE;
else
sigterm_received = BOOL_TRUE;
(The SIGINT param is needed because it's optional whether a
dialog handles a particular signal.)
Otherwise we'd race between main's use of sigterm_received and
the dialog's need to use it.
Do you prefer this over the signal handler swap?
Btw, I looked over the main sighandler a bit, and it seems like most
of it can simply go away for a pthreaded variant - there is no need
to handle SIGCLD, and all threads have the same pid, so basically
what is left is SIGHUP/SIGTERM/SIGINT/SIGQUIT handling, which does
nothing but a dlog_desist in most cases and setting the sigfoo_received
variable.
Yes, the previous patch in this series takes care of that. :)
Bill
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
