Date: Mon, 22 Nov 2010 11:13:06 +0100 From: Alexander Leidinger <netchild@FreeBSD.org> To: Kostik Belousov <kostikbel@gmail.com> Cc: svn-src-head@FreeBSD.org, svn-src-all@FreeBSD.org, src-committers@FreeBSD.org Subject: Re: svn commit: r215664 - in head/sys: compat/linux kern Message-ID: <20101122111306.16504je0tt7xe5us@webmail.leidinger.net> In-Reply-To: <20101122093134.GU2392@deviant.kiev.zoral.com.ua> References: <201011220907.oAM970To084256@svn.freebsd.org> <20101122093134.GU2392@deviant.kiev.zoral.com.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
Quoting Kostik Belousov <kostikbel@gmail.com> (from Mon, 22 Nov 2010 11:31:34 +0200): > On Mon, Nov 22, 2010 at 09:07:00AM +0000, Alexander Leidinger wrote: >> Author: netchild >> Date: Mon Nov 22 09:06:59 2010 >> New Revision: 215664 >> URL: http://svn.freebsd.org/changeset/base/215664 >> >> Log: >> By using the 32-bit Linux version of Sun's Java Development Kit 1.6 >> on FreeBSD (amd64), invocations of "javac" (or "java") eventually >> end with the output of "Killed" and exit code 137. >> @@ -196,6 +198,12 @@ linux_proc_exit(void *arg __unused, stru >> } else >> EMUL_SHARED_WUNLOCK(&emul_shared_lock); >> >> + if ((shared_flags & EMUL_SHARED_HASXSTAT) != 0) { >> + PROC_LOCK(p); >> + p->p_xstat = shared_xstat; >> + PROC_UNLOCK(p); >> + } > Why is process lock taken there ? The assignment to u_short inside the > properly aligned structure is atomic on all supported architectures, and > the thread that should see side-effect of assignment is the same thread > that does assignment. Change below. >> + >> if (child_clear_tid != NULL) { >> struct linux_sys_futex_args cup; >> int null = 0; >> @@ -257,6 +265,9 @@ linux_proc_exec(void *arg __unused, stru >> if (__predict_false(imgp->sysent == &elf_linux_sysvec >> && p->p_sysent != &elf_linux_sysvec)) >> linux_proc_init(FIRST_THREAD_IN_PROC(p), p->p_pid, 0); >> + if (__predict_false(p->p_sysent == &elf_linux_sysvec)) >> + /* Kill threads regardless of imgp->sysent value */ >> + linux_kill_threads(FIRST_THREAD_IN_PROC(p), SIGKILL); > This is better expressed by > if ((p->p_sysent->sv_flags & SV_ABI_MASK) == SV_ABI_LINUX) Is this OK for you? ---snip--- Index: compat/linux/linux_emul.c =================================================================== --- compat/linux/linux_emul.c (Revision 215664) +++ compat/linux/linux_emul.c (Arbeitskopie) @@ -198,11 +198,8 @@ } else EMUL_SHARED_WUNLOCK(&emul_shared_lock); - if ((shared_flags & EMUL_SHARED_HASXSTAT) != 0) { - PROC_LOCK(p); + if ((shared_flags & EMUL_SHARED_HASXSTAT) != 0) p->p_xstat = shared_xstat; - PROC_UNLOCK(p); - } if (child_clear_tid != NULL) { struct linux_sys_futex_args cup; @@ -265,7 +262,8 @@ if (__predict_false(imgp->sysent == &elf_linux_sysvec && p->p_sysent != &elf_linux_sysvec)) linux_proc_init(FIRST_THREAD_IN_PROC(p), p->p_pid, 0); - if (__predict_false(p->p_sysent == &elf_linux_sysvec)) + if (__predict_false((p->p_sysent->sv_flags & SV_ABI_MASK) == + SV_ABI_LINUX)) /* Kill threads regardless of imgp->sysent value */ linux_kill_threads(FIRST_THREAD_IN_PROC(p), SIGKILL); if (__predict_false(imgp->sysent != &elf_linux_sysvec ---snip--- > Regardless of this mostly cosmetic issue, this is racy. Other > linux thread in the same process might do an execve(3). > More, if execve(3) call fails, then you return into the process > that lacks all threads except the one that called execve(3). How critical is this in your opinion (relative to the issue this patch is fixing)? Do you prefer a backout or do you think the probability that the someone wins the race is low enough? Do you see a solution for the race? Bye, Alexander. -- http://www.Leidinger.net Alexander @ Leidinger.net: PGP ID = B0063FE7 http://www.FreeBSD.org netchild @ FreeBSD.org : PGP ID = 72077137 This generation doesn't have emotional baggage. We have emotional moving vans. -- Bruce Feirstein
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20101122111306.16504je0tt7xe5us>