Re: [PATCH 4/4] xfsdump: convert to the POSIX signal API

[Bill Kendall <wkendall@sgi.com>]

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@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs