Re: Kernel threads

[Oleg Nesterov <oleg@tv-sign.ru>]

On 03/08, Roland McGrath wrote:
>
> Your change seems fine to me.  I certainly concur that it seems insane
> for init to be responsible for tasks created magically inside the
> kernel.  The history I've found says that the setting to SIGCHLD was
> introduced as part of "v2.5.1.9 -> v2.5.1.10", without detailed
> commentary in the log.  This was probably before the auto-reaping
> semantics worked as they do now.  So like the man said, at the time,
> it seemed the logical thing to do.
>
> To be paranoid, I wouldn't make this change in any stable kernel series.
> It changes behavior visible to userland (init) from how it has been
> consistently for five years, so, who knows, something might notice.
> The old behavior is pretty harmless, albeit changing it seems both
> preferable and harmless.

Yes sure, this change shoud be tested in -mm tree (I'll send the patch
on Sunday after some testing). The only (afaics) problem is that with
this change a kernel thread must not do do_fork(CLONE_THREAD). I think
it should not, but currently this is technically possible. Perhaps it
makes sense to add BUG_ON(CLONE_THREAD && group_leader->exit_signal==-1)
in copy_process().

On a related note,

	zap_other_threads:

		if (t != p->group_leader)
			t->exit_signal = -1;

looks like another leftover to me, we already depend on the fact that
all sub-threads have ->exit_signal == -1 (otherwise, for example, a
thread group just can't exit properly).


While we are talking about kernel threads, there is something I can't
undestand. kthread/daemonize use sigprocmask(SIG_BLOCK) to protect
against signals. This doesn't look right to me, because this doesn't
prevent the signal delivery, this only blocks signal_wake_up(). Every
"killall -33 khelper" means a "struct siginfo" leak.

Imho, the kernel thread shouldn't play with ->blocked at all. Instead
it should set SIG_IGN for all handlers. If it really needs, say, SIGCHLD,
it should call allow_signal() anyway. Do you see any problems with this
approach?

Oleg.