notify_parent (was: Re: Linux 2.6.9-rc2)

notify_parent (was: Re: Linux 2.6.9-rc2)

[Geert Uytterhoeven]

On Mon, 13 Sep 2004, Linus Torvalds wrote:
> Roland McGrath:
>   o cleanup ptrace stops and remove notify_parent

However, there are still a few users of notify_parent():

| arch/m68k/kernel/signal.c:			notify_parent(current, SIGCHLD);
| arch/m68k/kernel/signal.c:					notify_parent(current, SIGCHLD);
| arch/ia64/kernel/signal.c:		 * get_signal_to_deliver() may have run a debugger (via notify_parent())
| arch/parisc/kernel/ptrace.c:			//notify_parent(child, SIGCHLD);
| arch/m68knommu/kernel/signal.c:			notify_parent(current, SIGCHLD);
| arch/m68knommu/kernel/signal.c:					notify_parent(current, SIGCHLD);
| arch/h8300/kernel/signal.c:			notify_parent(current, SIGCHLD);
| arch/h8300/kernel/signal.c:                                        notify_parent(current, SIGCHLD);

Any suggestion how to convert them?

Thanks!

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: notify_parent (was: Re: Linux 2.6.9-rc2)

[Roland McGrath]

> On Mon, 13 Sep 2004, Linus Torvalds wrote:
> > Roland McGrath:
> >   o cleanup ptrace stops and remove notify_parent
> 
> However, there are still a few users of notify_parent():

IIRC all these are old arch-specific signal code that is rampantly wrong in
semantics compared to what the up-to-date arch's using the generic code do,
for quite a long time now.  I believe I mentioned this when I posted the
patch.  All this arch signal code needs to be rewritten to use
get_signal_to_deliver, and define ptrace_signal_deliver appropriately to
get its arch-specific work done.  The old style of code that does all the
central signal dispatch logic itself is hopeless.  Mostly you have a lot of
old cruft to remove, and the code that needs to be left is much smaller and
simpler because the complex stuff is in the shared kernel/signal.c.

> You forgot to add `struct rusage _rusage' members to struct siginfo._sigchld
> for 2 architectures that define HAVE_ARCH_SIGINFO_T.

I didn't forget.  I never claimed to update other arch's.  The arch
maintainers need to keep up with patches like you are doing with these
here.  Most other arch maintainers have submitted their updates already.
You may need to update some copy_*_siginfo functions if your arch's
have them in compat code as well.



Thanks,
Roland

Re: notify_parent (was: Re: Linux 2.6.9-rc2)

[Geert Uytterhoeven]

On Tue, 14 Sep 2004, Roland McGrath wrote:
> > On Mon, 13 Sep 2004, Linus Torvalds wrote:
> > > Roland McGrath:
> > >   o cleanup ptrace stops and remove notify_parent
> >
> > However, there are still a few users of notify_parent():
>
> IIRC all these are old arch-specific signal code that is rampantly wrong in
> semantics compared to what the up-to-date arch's using the generic code do,
> for quite a long time now.  I believe I mentioned this when I posted the
> patch.  All this arch signal code needs to be rewritten to use
> get_signal_to_deliver, and define ptrace_signal_deliver appropriately to
> get its arch-specific work done.  The old style of code that does all the
> central signal dispatch logic itself is hopeless.  Mostly you have a lot of
> old cruft to remove, and the code that needs to be left is much smaller and
> simpler because the complex stuff is in the shared kernel/signal.c.
>
> > You forgot to add `struct rusage _rusage' members to struct siginfo._sigchld
> > for 2 architectures that define HAVE_ARCH_SIGINFO_T.
>
> I didn't forget.  I never claimed to update other arch's.  The arch
> maintainers need to keep up with patches like you are doing with these
> here.  Most other arch maintainers have submitted their updates already.
> You may need to update some copy_*_siginfo functions if your arch's
> have them in compat code as well.

Hence (IMHO) they should have gone through linux-arch...

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: notify_parent (was: Re: Linux 2.6.9-rc2)

[Geert Uytterhoeven]

On Tue, 14 Sep 2004, Roland McGrath wrote:
> > On Mon, 13 Sep 2004, Linus Torvalds wrote:
> > > Roland McGrath:
> > >   o cleanup ptrace stops and remove notify_parent
> > 
> > However, there are still a few users of notify_parent():
> 
> IIRC all these are old arch-specific signal code that is rampantly wrong in
> semantics compared to what the up-to-date arch's using the generic code do,
> for quite a long time now.  I believe I mentioned this when I posted the
> patch.  All this arch signal code needs to be rewritten to use
> get_signal_to_deliver, and define ptrace_signal_deliver appropriately to
> get its arch-specific work done.  The old style of code that does all the
> central signal dispatch logic itself is hopeless.  Mostly you have a lot of
> old cruft to remove, and the code that needs to be left is much smaller and
> simpler because the complex stuff is in the shared kernel/signal.c.

Can anyone with more intimate knowledge of (m68k) signal handling please take a
look at this? I gave it a first try, but I don't know enough about the code in
question to guarantee anything beyond `it does compile again'.

Thanks!

--- linux-2.6.9-rc2/arch/m68k/kernel/signal.c	2004-09-13 21:57:25.000000000 +0200
+++ linux-m68k-2.6.9-rc2/arch/m68k/kernel/signal.c	2004-09-19 22:01:26.000000000 +0200
@@ -42,6 +42,7 @@
 #include <linux/personality.h>
 #include <linux/tty.h>
 #include <linux/binfmts.h>
+#include <linux/suspend.h>
 
 #include <asm/setup.h>
 #include <asm/uaccess.h>
@@ -990,128 +991,34 @@
 asmlinkage int do_signal(sigset_t *oldset, struct pt_regs *regs)
 {
 	siginfo_t info;
-	struct k_sigaction *ka;
+	int signr;
+	struct k_sigaction ka;
 
 	current->thread.esp0 = (unsigned long) regs;
 
+	if (current->flags & PF_FREEZE) {
+		refrigerator(0);
+		goto no_signal;
+	}
+
 	if (!oldset)
 		oldset = &current->blocked;
 
-	for (;;) {
-		int signr;
-
-		signr = dequeue_signal(current, &current->blocked, &info);
-
-		if (!signr)
-			break;
-
-		if ((current->ptrace & PT_PTRACED) && signr != SIGKILL) {
-			current->exit_code = signr;
-			current->state = TASK_STOPPED;
-			regs->sr &= ~PS_T;
-
-			/* Did we come from a system call? */
-			if (regs->orig_d0 >= 0) {
-				/* Restart the system call the same way as
-				   if the process were not traced.  */
-				struct k_sigaction *ka =
-					&current->sighand->action[signr-1];
-				int has_handler =
-					(ka->sa.sa_handler != SIG_IGN &&
-					 ka->sa.sa_handler != SIG_DFL);
-				handle_restart(regs, ka, has_handler);
-			}
-			notify_parent(current, SIGCHLD);
-			schedule();
-
-			/* We're back.  Did the debugger cancel the sig?  */
-			if (!(signr = current->exit_code)) {
-			discard_frame:
-			    /* Make sure that a faulted bus cycle isn't
-			       restarted (only needed on the 680[23]0).  */
-			    if (regs->format == 10 || regs->format == 11)
-				regs->stkadj = frame_extra_sizes[regs->format];
-			    continue;
-			}
-			current->exit_code = 0;
-
-			/* The debugger continued.  Ignore SIGSTOP.  */
-			if (signr == SIGSTOP)
-				goto discard_frame;
-
-			/* Update the siginfo structure.  Is this good?  */
-			if (signr != info.si_signo) {
-				info.si_signo = signr;
-				info.si_errno = 0;
-				info.si_code = SI_USER;
-				info.si_pid = current->parent->pid;
-				info.si_uid = current->parent->uid;
-				info.si_uid16 = high2lowuid(current->parent->uid);
-			}
-
-			/* If the (new) signal is now blocked, requeue it.  */
-			if (sigismember(&current->blocked, signr)) {
-				send_sig_info(signr, &info, current);
-				continue;
-			}
-		}
-
-		ka = &current->sighand->action[signr-1];
-		if (ka->sa.sa_handler == SIG_IGN) {
-			if (signr != SIGCHLD)
-				continue;
-			/* Check for SIGCHLD: it's special.  */
-			while (sys_wait4(-1, NULL, WNOHANG, NULL) > 0)
-				/* nothing */;
-			continue;
-		}
-
-		if (ka->sa.sa_handler == SIG_DFL) {
-			int exit_code = signr;
-
-			if (current->pid == 1)
-				continue;
-
-			switch (signr) {
-			case SIGCONT: case SIGCHLD:
-			case SIGWINCH: case SIGURG:
-				continue;
-
-			case SIGTSTP: case SIGTTIN: case SIGTTOU:
-				if (is_orphaned_pgrp(process_group(current)))
-					continue;
-				/* FALLTHRU */
-
-			case SIGSTOP: {
-				struct sighand_struct *sighand;
-				current->state = TASK_STOPPED;
-				current->exit_code = signr;
-				sighand = current->parent->sighand;
-				if (sighand && !(sighand->action[SIGCHLD-1].sa.sa_flags
-					     & SA_NOCLDSTOP))
-					notify_parent(current, SIGCHLD);
-				schedule();
-				continue;
-			}
-
-			case SIGQUIT: case SIGILL: case SIGTRAP:
-			case SIGIOT: case SIGFPE: case SIGSEGV:
-			case SIGBUS: case SIGSYS: case SIGXCPU: case SIGXFSZ:
-				if (do_coredump(signr, exit_code, regs))
-					exit_code |= 0x80;
-				/* FALLTHRU */
-
-			default:
-				do_group_exit(signr);
-				/* NOTREACHED */
-			}
-		}
+	signr = get_signal_to_deliver(&info, &ka, regs, NULL);
+	if (signr > 0) {
+		/* Reenable any watchpoints before delivering the
+		 * signal to user space. The processor register will
+		 * have been cleared if the watchpoint triggered
+		 * inside the kernel.
+		 */
+		regs->sr &= ~PS_T;
 
 		/* Whee!  Actually deliver the signal.  */
-		handle_signal(signr, ka, &info, oldset, regs);
+		handle_signal(signr, &ka, &info, oldset, regs);
 		return 1;
 	}
 
+no_signal:
 	/* Did we come from a system call? */
 	if (regs->orig_d0 >= 0)
 		/* Restart the system call - no handlers present */
--- linux-2.6.9-rc2/include/asm-m68k/siginfo.h	2004-04-27 20:42:20.000000000 +0200
+++ linux-m68k-2.6.9-rc2/include/asm-m68k/siginfo.h	2004-09-19 12:52:57.000000000 +0200
@@ -46,6 +46,8 @@
 			clock_t _utime;
 			clock_t _stime;
 			__kernel_uid32_t _uid32; /* sender's uid */
+			// FIXME to be removed again in 2.6.9-rc3
+			struct rusage _rusage;
 		} _sigchld;
 
 		/* SIGILL, SIGFPE, SIGSEGV, SIGBUS */
--- linux-2.6.9-rc2/include/asm-m68k/signal.h	2004-05-24 11:13:53.000000000 +0200
+++ linux-m68k-2.6.9-rc2/include/asm-m68k/signal.h	2004-09-19 22:00:15.000000000 +0200
@@ -213,7 +213,7 @@
 	return word ^ 31;
 }
 
-#define HAVE_ARCH_GET_SIGNAL_TO_DELIVER
+#define ptrace_signal_deliver(regs, cookie) do { } while (0)
 
 #endif /* __KERNEL__ */
 

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: notify_parent (was: Re: Linux 2.6.9-rc2)

[Andreas Schwab]

Geert Uytterhoeven <geert@linux-m68k.org> writes:

> -			regs->sr &= ~PS_T;
> -
> -			/* Did we come from a system call? */
> -			if (regs->orig_d0 >= 0) {
> -				/* Restart the system call the same way as
> -				   if the process were not traced.  */
> -				struct k_sigaction *ka =
> -					&current->sighand->action[signr-1];
> -				int has_handler =
> -					(ka->sa.sa_handler != SIG_IGN &&
> -					 ka->sa.sa_handler != SIG_DFL);
> -				handle_restart(regs, ka, has_handler);
> -			}

This should be put in ptrace_signal_deliver.  That had fixed quite a few
gdb testsuite failures.

> -			/* We're back.  Did the debugger cancel the sig?  */
> -			if (!(signr = current->exit_code)) {
> -			discard_frame:
> -			    /* Make sure that a faulted bus cycle isn't
> -			       restarted (only needed on the 680[23]0).  */
> -			    if (regs->format == 10 || regs->format == 11)
> -				regs->stkadj = frame_extra_sizes[regs->format];

This is important if you want continue after a SEGV.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: notify_parent (was: Re: Linux 2.6.9-rc2)

[Geert Uytterhoeven]

On Mon, 20 Sep 2004, Andreas Schwab wrote:
> Geert Uytterhoeven <geert@linux-m68k.org> writes:
> 
> > -			regs->sr &= ~PS_T;
> > -
> > -			/* Did we come from a system call? */
> > -			if (regs->orig_d0 >= 0) {
> > -				/* Restart the system call the same way as
> > -				   if the process were not traced.  */
> > -				struct k_sigaction *ka =
> > -					&current->sighand->action[signr-1];
> > -				int has_handler =
> > -					(ka->sa.sa_handler != SIG_IGN &&
> > -					 ka->sa.sa_handler != SIG_DFL);
> > -				handle_restart(regs, ka, has_handler);
> > -			}
> 
> This should be put in ptrace_signal_deliver.  That had fixed quite a few
> gdb testsuite failures.

OK.

> 
> > -			/* We're back.  Did the debugger cancel the sig?  */
> > -			if (!(signr = current->exit_code)) {
> > -			discard_frame:
> > -			    /* Make sure that a faulted bus cycle isn't
> > -			       restarted (only needed on the 680[23]0).  */
> > -			    if (regs->format == 10 || regs->format == 11)
> > -				regs->stkadj = frame_extra_sizes[regs->format];
> 
> This is important if you want continue after a SEGV.

IC. But where should I do that?

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: notify_parent (was: Re: Linux 2.6.9-rc2)

[Andreas Schwab]

Geert Uytterhoeven <geert@linux-m68k.org> writes:

>> > -			/* We're back.  Did the debugger cancel the sig?  */
>> > -			if (!(signr = current->exit_code)) {
>> > -			discard_frame:
>> > -			    /* Make sure that a faulted bus cycle isn't
>> > -			       restarted (only needed on the 680[23]0).  */
>> > -			    if (regs->format == 10 || regs->format == 11)
>> > -				regs->stkadj = frame_extra_sizes[regs->format];
>> 
>> This is important if you want continue after a SEGV.
>
> IC. But where should I do that?

Looks like we need a hook in the generic code.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Maxfeldstraße 5, 90409 Nürnberg, Germany
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."