Re: [PATCH] multipathd: fix inverted signal blocking logic

[Martin Wilck <mwilck@suse.com>]

On Fri, 2018-03-02 at 23:27 +0000, Bart Van Assche wrote:
> On Sat, 2018-03-03 at 00:16 +0100, Martin Wilck wrote:
> > On Fri, 2018-03-02 at 22:23 +0000, Bart Van Assche wrote:
> > > On Fri, 2018-03-02 at 23:15 +0100, Martin Wilck wrote:
> > > > On Fri, 2018-03-02 at 21:35 +0000, Bart Van Assche wrote:
> > > > > This change looks more complicated to me than necessary. Have
> > > > > you
> > > > > considered
> > > > > to pass an empty signal set as the fourth ppoll() argument?
> > > > 
> > > > An empty set would mean that no signal is blocked during
> > > > ppoll().
> > > > Therefore e.g. SIGALRM would terminate multipathd if it arrives
> > > > during the ppoll (no handler set, and default action is
> > > > "Term").
> > > 
> > > Have you considered to only block SIGALRM while ppoll() is in
> > > progress?
> > 
> > Why should we? The same reasoning applies to other signals such as
> > e.g.
> > SIGUSR2. We need to block all signals except those that we can
> > handle.
> 
> I see two reasons:
> - All signals that are delivered to the multipathd process except
> SIGALRM
>   should interrupt ppoll(). SIGPIPE is delivered to the thread that
> caused
>   that signal to be raised. SIGUSR2 is sent to a specific thread.
> Hence
>   neither SIGPIPE nor SIGUSR2 will ever be delivered to the thread
> that
>   calls ppoll().

So, there's no reason not to block them, right? Is it expected behavior
that a user running 'kill -USR2 $(pidof multipathd)' terminates the
process? Why do you think these signals should interrupt ppoll()
although the uxlsnr can't can't handle them? Isn't it sufficient that
they're handled by the threads that are meant to do that?

IMO the idea of ppoll() is to specify a set of signals the calling
thread is interested in, and that's precisely the set of signals
handle_signals() can deal with.

I'm rather neutral about SIGPIPE, we can add a sigdelset(&set, SIGPIPE)
if you insist (multipathd will die anyway), but clearing SIGUSR2 in the
uxlsnr would be wrong IMO, as it's dedicated to the waiter thread.

> - Asking ppoll() to block all signals is weird because some signals,
> e.g.
>   SIGKILL and SIGSTOP, cannot be blocked.

And the kernel makes sure that doesn't happen. So I can add
sigdelset(&set, SIGKILL) bit it's kind of pointless.

Martin

> 
> Bart.

-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel