lkml.org 
[lkml]   [2011]   [Apr]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 4/6] signal: sigprocmask() should do retarget_shared_pending()
    On 04/12, Linus Torvalds wrote:
    >
    > On Mon, Apr 11, 2011 at 10:21 AM, Oleg Nesterov <oleg@redhat.com> wrote:
    > >
    > > I am not sure this is bug, but at least this looks strange imho. T1 should
    > > not sleep forever, there is a signal which should wake it up.
    >
    > Hmm. I worry about the overhead of this, and I'm not 100% convinced we need it.

    Indeed. That is why RFC.

    I simply do not know if this is buggy or not. I reported this oddity a long
    ago, but I can't recall the result of discussion (or it was ignored ?).

    And I do not like the fact we need a lot of changes, albeit trivial. We should
    convert almost every code which changes current->blocked. Otoh, perhaps this
    makes sense by itself...

    > > --- sigprocmask/include/linux/signal.h~4_sigprocmask_retarget   2011-04-06 21:33:50.000000000 +0200
    > > +++ sigprocmask/include/linux/signal.h  2011-04-11 18:16:51.000000000 +0200
    > > @@ -2131,6 +2131,11 @@ int sigprocmask(int how, sigset_t *set,
    > >        }
    > >
    > >        spin_lock_irq(&tsk->sighand->siglock);
    > > +       if (signal_pending(tsk) && !thread_group_empty(tsk)) {
    > > +               sigset_t not_newblocked;
    > > +               signorsets(&not_newblocked, &current->blocked, &newset);
    > > +               retarget_shared_pending(tsk, &not_newblocked);
    > > +       }
    > >        tsk->blocked = newset;
    > >        recalc_sigpending();
    > >        spin_unlock_irq(&tsk->sighand->siglock);
    >
    > I absolutely detest how you made "sigprocmask()" the main interface to
    > do this all, and then add new callers.
    >
    > It's a horrid interface with that crazy "how" argument, and comes out
    > of the user-space system call interface. If we make kernel users do
    > this, especially critical ones like the signal handling code, please
    > just extract out just the actual "set new signal mask" part.
    >
    > So please just introduce a "sig_set_blocked()" or something, without
    > the crazy "switch (how)" crud, and make sigprocmask() and everybody
    > else use _that_ instead.

    You know, initially I did exactly this. set_current_blocked() was its
    name. But then I noticed that handle_signal() can naturally use SIG_BLOCK,
    sigtimedwait() could use SIG_UNBLOCK...

    Nevermind,

    > That would make me much happier about the patch series, I suspect.

    OK. I'll redo and resend.

    Oleg.

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2011-04-14 21:39    [W:0.023 / U:60.224 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site