Re: [PATCH] Improve LinuxThreads compatibility in rfork()

On Mon, Jul 11, 2011 at 03:27:56PM +0200, Petr Salinger wrote:
> >>This patch made by Petr Salinger improves compatibility with
> >>LinuxThreads in rfork() syscall.  The Linux clone() implementation
> >>allows specifying the signal sent to parent when child terminates
> >>(instead of SIGCHLD).
> >>
> >>As the threading implementation in Debian GNU/kFreeBSD is
> >>LinuxThreads-based, we had to diverge from upstream kFreeBSD ABI and
> >>implement this extension.
> >>
> >>I hope it is acceptable for you to use the same encoding, this way we
> >>would archieve ABI compatibility to run Debian GNU/kFreeBSD inside a
> >>chroot/jail on top of a FreeBSD system.
> >>
> >>Thanks for considering
> >>
> >>--
> >>Robert Millan
> >
> >>--- a/sys/kern/kern_fork.c
> >>+++ b/sys/kern/kern_fork.c
> >>_at__at_ -477,7 +477,13 _at__at_
> >> 		p2->p_sigacts = newsigacts;
> >> 	}
> >> 	if (flags & RFLINUXTHPN)
> >>-	        p2->p_sigparent = SIGUSR1;
> >>+	{
> >>+		int sig;
> >>+		sig = RFTHPNSIGNUM(flags);
> >>+	        if (sig == 0) sig = SIGUSR1;
> >>+	        if (sig == SIGCHLD) sig = 0;
> >>+	        p2->p_sigparent = sig;
> >>+	}
> >> 	else
> >> 	        p2->p_sigparent = SIGCHLD;
> >>
> >>--- a/sys/sys/unistd.h
> >>+++ b/sys/sys/unistd.h
> >>_at__at_ -182,6 +182,10 _at__at_
> >> #define	RFHIGHPID	(1<<18)	/* use a pid higher than 10 
> >> (idleproc) */
> >> #define	RFPPWAIT	(1<<31)	/* parent sleeps until child exits 
> >> (vfork) */
> >> #define	RFKERNELONLY	(RFSTOPPED | RFHIGHPID | RFPPWAIT)
> >>+#define RFTHPNSHIFT	24	/* reserve bits 24-30 */
> >>+#define RFTHPNMASK	0x7F    /* for compatibility with 
> >>linuxthreads/clone()   */
> >>+				/* allow to specify  "clone exit parent 
> >>notification" signal */
> >>+#define RFTHPNSIGNUM(flags)	(((flags) >> RFTHPNSHIFT) & RFTHPNMASK)
> >>
> >> #endif /* __BSD_VISIBLE */
> >>
> >I looked at this patch some time ago already.
> >
> >Can you, please, describe the reasoning behind the
> >>+	        if (sig == SIGCHLD) sig = 0;
> >line ?
> 
> The main reason is backward compatibility.
> The original FreeBSD code allows only to select between
> SIGUSR1 or SIGCHLD signals.
> 
> The our extension changes meaning of RFLINUXTHPN to select signal based on 
> bits 24-30 of passed flags instead of using SIGUSR1 every time.
> 
> When the passed "signal" number is zero, it should behave identically
> on plain FreeBSD and in our environment, therefore SIGUSR1 is selected.
> The assumption is (have been) that (yet) undefined bits are zero.
> That way we are backward compatible with original FreeBSD.
> 
> We still need an alternative way to select "none signal is sent"
> after child exit (under linux #0 is used).
> 
> The SIGCHLD can be "selected" (also on original FreeBSD) by not specifying 
> RFLINUXTHPN, therefore combination of RFLINUXTHPN and passed "signal" 
> number SIGCHLD is (have been) used for "none signal".
> 
> BTW, the opposite side is in
> 
> http://anonscm.debian.org/viewvc/glibc-bsd/trunk/glibc-ports/kfreebsd/clone.c?view=markup

I shall state that the sig == SIGCHLD case is ugly. Having the separate
flag "do not send signal to the parent" would be much less clumsy.
What are the requirements for the ABI stability for Debian/kFreeBSD ?
Can this be fixed now, or is it too late ?

Would you care to update the rfork(2) man page ?
Also, it would be ideal to reformat the kern_fork.c change according
to our style(9).