Re: [PATCH 0/3] coredump: fix the ancient signal problems
From: Linus Torvalds
Date: Sun Feb 17 2013 - 14:34:47 EST
On Sun, Feb 17, 2013 at 11:18 AM, Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
> Linus, et al, could you please ack/nack the intent? Of course I will
> appreciate if you can review the code, but what I am actually worried
> about is the user-visible change: the coredumping becomes killable but
> only by the _explicit_ SIGKILL, other fatal signals are "ignored".
That isn't a problem. In fact, we already have logic that makes the
act of writing a file be killable by SIGKILL (because you really
really want that for network filesystems, for example), so I suspect
that core-dumping was interruptible by SIGKILL even before you made it
explicitly so - simply because the IO itself was.
And even if it wasn't (because maybe the SIGKILL logic doesn't get
triggered due to all the special-case core-dumping code in signal
handling), SIGKILL really is very very special. Having it kill a
coredump in progress sounds fine to me. SIGKILL is used by
users/admins as a last-case thing to get rid of processes that are
problematic for the system, and we should really try to honor it over
just about anything else for that reason. It's the user saying "get
this thing away from me", we should honor it.
That said, I'm not convinced about your particular split of patches.
The first patch introduces that new SIGNAL_GROUP_COREDUMP, and then
the second patch modifies one of the new use cases:
- tsk->signal->flags |= SIGNAL_GROUP_COREDUMP;
+ tsk->signal->flags = SIGNAL_GROUP_COREDUMP;
and that just smells to me like you tried too hard to split things
into two patches.
And obviously, it will need more testing and review. I wonder if Al
Viro hould be on the cc. He doesn't always get involved, but when it
comes to core-dumping and signal handling, he *sometimes* does, and so
just checking if he has any comments might be worth it.
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/