User visible change.
do_coredump() kills all threads which share the same ->mm but only
the coredumping process gets the proper exit_code. Other tasks which
share the same ->mm die "silently" and return status == 0 to parent.
This is historical behaviour, not actually a bug. But I think Frank
Heckenbach rightly dislikes the current behaviour. Simple test-case:
#include <stdio.h>
#include <unistd.h>
#include <signal.h>
#include <sys/wait.h>
int main(void)
{
int stat;
if (!fork()) {
if (!vfork())
kill(getpid(), SIGQUIT);
}
wait(&stat);
printf("stat=%x\n", stat);
return 0;
}
Before this patch it prints "stat=0" despite the fact the child was
killed by SIGQUIT. After this patch the output is "stat=3" which
obviously makes more sense.
Even with this patch, only the task which originates the coredumping
gets "|= 0x80" if the core was actually dumped, but at least the
coredumping signal is visible to do_wait/etc.
Reported-by: Frank Heckenbach <[email protected]>
Signed-off-by: Oleg Nesterov <[email protected]>
---
fs/exec.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
--- V1/fs/exec.c~CD_STATUS 2009-12-18 00:20:50.000000000 +0100
+++ V1/fs/exec.c 2010-02-07 17:28:24.000000000 +0100
@@ -1536,12 +1536,13 @@ out:
return ispipe;
}
-static int zap_process(struct task_struct *start)
+static int zap_process(struct task_struct *start, int exit_code)
{
struct task_struct *t;
int nr = 0;
start->signal->flags = SIGNAL_GROUP_EXIT;
+ start->signal->group_exit_code = exit_code;
start->signal->group_stop_count = 0;
t = start;
@@ -1566,8 +1567,7 @@ static inline int zap_threads(struct tas
spin_lock_irq(&tsk->sighand->siglock);
if (!signal_group_exit(tsk->signal)) {
mm->core_state = core_state;
- tsk->signal->group_exit_code = exit_code;
- nr = zap_process(tsk);
+ nr = zap_process(tsk, exit_code);
}
spin_unlock_irq(&tsk->sighand->siglock);
if (unlikely(nr < 0))
@@ -1616,7 +1616,7 @@ static inline int zap_threads(struct tas
if (p->mm) {
if (unlikely(p->mm == mm)) {
lock_task_sighand(p, &flags);
- nr += zap_process(p);
+ nr += zap_process(p, exit_code);
unlock_task_sighand(p, &flags);
}
break;
On Mon, Feb 8, 2010 at 1:16 AM, Oleg Nesterov <[email protected]> wrote:> User visible change.>> do_coredump() kills all threads which share the same ->mm but only> the coredumping process gets the proper exit_code. Other tasks which> share the same ->mm die "silently" and return status == 0 to parent.>> This is historical behaviour, not actually a bug. But I think Frank> Heckenbach rightly dislikes the current behaviour. Simple test-case:>> #include <stdio.h>> #include <unistd.h>> #include <signal.h>> #include <sys/wait.h>>> int main(void)> {> int stat;>> if (!fork()) {> if (!vfork())> kill(getpid(), SIGQUIT);> }>> wait(&stat);> printf("stat=%x\n", stat);> return 0;> }>> Before this patch it prints "stat=0" despite the fact the child was> killed by SIGQUIT. After this patch the output is "stat=3" which> obviously makes more sense.>> Even with this patch, only the task which originates the coredumping> gets "|= 0x80" if the core was actually dumped, but at least the> coredumping signal is visible to do_wait/etc.
Nice changelog!
>> Reported-by: Frank Heckenbach <[email protected]>> Signed-off-by: Oleg Nesterov <[email protected]>
Acked-by: WANG Cong <[email protected]>
Thank you!
> --->> fs/exec.c | 8 ++++----> 1 file changed, 4 insertions(+), 4 deletions(-)>> --- V1/fs/exec.c~CD_STATUS 2009-12-18 00:20:50.000000000 +0100> +++ V1/fs/exec.c 2010-02-07 17:28:24.000000000 +0100> @@ -1536,12 +1536,13 @@ out:> return ispipe;> }>> -static int zap_process(struct task_struct *start)> +static int zap_process(struct task_struct *start, int exit_code)> {> struct task_struct *t;> int nr = 0;>> start->signal->flags = SIGNAL_GROUP_EXIT;> + start->signal->group_exit_code = exit_code;> start->signal->group_stop_count = 0;>> t = start;> @@ -1566,8 +1567,7 @@ static inline int zap_threads(struct tas> spin_lock_irq(&tsk->sighand->siglock);> if (!signal_group_exit(tsk->signal)) {> mm->core_state = core_state;> - tsk->signal->group_exit_code = exit_code;> - nr = zap_process(tsk);> + nr = zap_process(tsk, exit_code);> }> spin_unlock_irq(&tsk->sighand->siglock);> if (unlikely(nr < 0))> @@ -1616,7 +1616,7 @@ static inline int zap_threads(struct tas> if (p->mm) {> if (unlikely(p->mm == mm)) {> lock_task_sighand(p, &flags);> - nr += zap_process(p);> + nr += zap_process(p, exit_code);> unlock_task_sighand(p, &flags);> }> break;>>????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay? ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?
That seems reasonable to me.
Thanks,
Roland