Re: [RFC + PATCH] signalfd simplification

On 09/01, Davide Libenzi wrote:

I'm playing at the moment with this patch, that recall Ben's idea of
attaching to the sighand only during read/poll, and calling dequeue_signal()
only with "current". This simplifies the signalfd logic quite a bit.
If this patch is applied, a task calling signalfd can read its own private
signals, and its own group signals.

fs/exec.c | 3
fs/signalfd.c | 186 +++++++---------------------------------------
include/linux/init_task.h | 2
include/linux/sched.h | 2
include/linux/signalfd.h | 29 -------
kernel/exit.c | 9 --
kernel/fork.c | 2
kernel/signal.c | 8 -
8 files changed, 40 insertions(+), 201 deletions(-)

Imho, very very nice. We lose the ability to read the cross-process signals,
but I doubt very much we should regret about that.

I cc'ed Michael, because it makes sense to document a user-visible change.
With this patch, the forked child reads its own signals (not parent's) via
the inherited signalfd (or if it was passed with unix socket).

Small problem: unless I missed something, signalfd_deliver() and sys_signalfd()
should use wake_up_all(), not wake_up() which implies nr_exclusive == 1.

It is possible that we have multiple threads waiting on ->signalfd_wqh with
the the different ->sigmask. In this case, the first woken thread can ignore
the signal, we should wake up all of them.

We can optimize this later, using a "clever" wait_queue_func_t if needed.

+ spin_lock_irq(&current->sighand->siglock);
+ if (next_signal(&current->pending, &ctx->sigmask) > 0 ||
+ next_signal(&current->signal->shared_pending,
+ &ctx->sigmask) > 0)

Very minor nit: next_signal() always returns the value >= 0, imho the "> 0"
check looks a bit confusing.

Oleg.

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

Relevant Pages

  • [PATCH 0/7][v8] Container-init signal semantics
    ... Patch 5/7 is new in this set and fixes a bug. ... Container-init must behave like global-init to processes within the ... within the container (i.e SIG_DFL signals that terminate the process). ... namespace of the sender but since signals can originate from workqueues/ ...
    (Linux-Kernel)
  • Re: [PATCH] cleanup ptrace stops and remove notify_parent
    ... > It clashes significantly with your waitid syscall patch. ... static int may_ptrace_attach ... extern int kill_pg_info(int, struct siginfo *, pid_t); ... Handle magic process-wide effects of stop/continue signals. ...
    (Linux-Kernel)
  • Re: main thread pthread_exit/sys_exit bug!
    ... I'll try to re-do and re-send this patch this week. ... continue to handle the signals until the while group exits, ... and wants to attach gdb to it? ... And I think gdb is wrong. ...
    (Linux-Kernel)
  • [patches] [more problems] Re: unkillable process stuck in "nfsaio"
    ... could you, please, test this little patch: ... (stuck in nfsaio) ... for signals delivered to it. ...
    (freebsd-current)
  • Re: [PATCH] Convert sigaction to act like other unices
    ... >> Here's a patch that converts all architectures to behave like other unix ... A patch entry has to ... Finally, it was ask, what do other unix boxes do? ... to signals and SA_NODEFER. ...
    (Linux-Kernel)