Message ID | 1349444664.3638.46.camel@dabdike.int.hansenpartnership.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
On Fri, Oct 05, 2012 at 02:44:24PM +0100, James Bottomley wrote: > On Fri, 2012-10-05 at 12:07 +0100, James Bottomley wrote: > > I tried out the code at > > > > git://git.kernel.org/pub/scm/linux/kernel/git/viro/signal.git > > experimental-kernel_thread > > > > and it gives me this panic on boot. > > OK, found the fix: the idle thread is a kernel thread, but it doesn't > come through kernel_thread(). The fix is to check for it (fortunately > it has the signal usp == 0). Um... I see, but I really wonder if that's the right fix. FWIW, sparc will have the same problem... Hell knows. OTOH, it's a nice way to get of implicit interplay between copy_thread() and idle_regs() - note that SMP architectures doing default idle_regs() need to be damn careful about what they do in their "is that kernel thread" logics; all-zeros pt_regs might give varying results on user_mode(regs) tests, etc. Might be better to go for if (p->flags & PF_KTHREAD) { if (!usp) { we are starting an idle thread } else { we are setting things up for kernel_thread() } } else { we are forking } kind of logics, looking at regs only in the last case. And to hell with (separate and overridable) idle_regs() once everything goes that way... > I'm now getting as freeing the init memory, which then hangs, so I > suspect some type of execve failure trying to start the initrd... I'm > debugging. What are you using for toolchain, BTW? With gcc 4.3 / binutils 2.20 (cross-builds) I'm getting panics from mainline kernel on parisc32 all way back to 2.6.28... -- To unsubscribe from this list: send the line "unsubscribe linux-parisc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2012-10-05 at 15:48 +0100, Al Viro wrote: > On Fri, Oct 05, 2012 at 02:44:24PM +0100, James Bottomley wrote: > > On Fri, 2012-10-05 at 12:07 +0100, James Bottomley wrote: > > > I tried out the code at > > > > > > git://git.kernel.org/pub/scm/linux/kernel/git/viro/signal.git > > > experimental-kernel_thread > > > > > > and it gives me this panic on boot. > > > > OK, found the fix: the idle thread is a kernel thread, but it doesn't > > come through kernel_thread(). The fix is to check for it (fortunately > > it has the signal usp == 0). > > Um... I see, but I really wonder if that's the right fix. FWIW, sparc > will have the same problem... Hell knows. OTOH, it's a nice way to > get of implicit interplay between copy_thread() and idle_regs() - note > that SMP architectures doing default idle_regs() need to be damn careful > about what they do in their "is that kernel thread" logics; all-zeros > pt_regs might give varying results on user_mode(regs) tests, etc. > Might be better to go for > if (p->flags & PF_KTHREAD) { > if (!usp) { > we are starting an idle thread > } else { > we are setting things up for kernel_thread() > } > } else { > we are forking > } > kind of logics, looking at regs only in the last case. And to hell with > (separate and overridable) idle_regs() once everything goes that way... But there's not a lot of point. forking an idle thread actually doesn't care about any of the register execution setup because it never really uses it to execute. That's why it was safe for us to use the user thread setup ... I suppose the interior of the kernel thread case could be conditioned on if (usp). > > I'm now getting as freeing the init memory, which then hangs, so I > > suspect some type of execve failure trying to start the initrd... I'm > > debugging. > > What are you using for toolchain, BTW? With gcc 4.3 / binutils 2.20 > (cross-builds) I'm getting panics from mainline kernel on parisc32 all > way back to 2.6.28... My toolchain is jejb@ion> gcc -v Using built-in specs. Target: hppa-linux-gnu Configured with: ../src/configure -v --with-pkgversion='Debian 4.4.5-8' --with-bugurl=file:///usr/share/doc/gcc-4.4/README.Bugs --enable-languages=c,c++,fortran,objc,obj-c++ --prefix=/usr --program-suffix=-4.4 --enable-shared --enable-multiarch --enable-linker-build-id --with-system-zlib --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --with-gxx-include-dir=/usr/include/c++/4.4 --libdir=/usr/lib --enable-nls --enable-clocale=gnu --enable-libstdcxx-debug --disable-libssp --enable-objc-gc --disable-libstdcxx-pch --enable-checking=release --build=hppa-linux-gnu --host=hppa-linux-gnu --target=hppa-linux-gnu Thread model: posix gcc version 4.4.5 (Debian 4.4.5-8) It's the one that was in debian testing before parisc got dumped James -- To unsubscribe from this list: send the line "unsubscribe linux-parisc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Oct 05, 2012 at 03:55:36PM +0100, James Bottomley wrote: > But there's not a lot of point. forking an idle thread actually doesn't > care about any of the register execution setup because it never really > uses it to execute. That's why it was safe for us to use the user > thread setup ... I suppose the interior of the kernel thread case could > be conditioned on if (usp). FWIW, I'm digging through the fork_idle() interplay with copy_thread() right now and the more I'm looking the less I like having called copy_thread() at all on that path. Note that copy_process() can trivially tell it from the normal (do_fork()) caller, just by checking if pid is non-NULL. And it has to check that anyway when it decides to generate the pid for child... Hell knows; I wonder how much of copy_thread() work is needed in that case... -- To unsubscribe from this list: send the line "unsubscribe linux-parisc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 5-Oct-12, at 10:48 AM, Al Viro wrote: > What are you using for toolchain, BTW? With gcc 4.3 / binutils 2.20 > (cross-builds) I'm getting panics from mainline kernel on parisc32 all > way back to 2.6.28... Current debian unstable tools are available in shirka.esiee.fr:/home/ dave/archives and shirka.esiee.fr:/home/dave/archives-new. It's pretty straight forward to update binutils, gcc, libc, etc. gcc 4.4, 4.5, 4.6 and 4.7 are available. In particular, 4.6 and 4.7 have significant bug fixes. James seems to have an account on shirka. I don't have any place to distribute the debs that are on shirka, or the time to remove obsolete builds. Any volunteers? Dave -- John David Anglin dave.anglin@bell.net -- To unsubscribe from this list: send the line "unsubscribe linux-parisc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Oct 05, 2012 at 03:55:36PM +0100, James Bottomley wrote: > On Fri, 2012-10-05 at 15:48 +0100, Al Viro wrote: > > On Fri, Oct 05, 2012 at 02:44:24PM +0100, James Bottomley wrote: > > > On Fri, 2012-10-05 at 12:07 +0100, James Bottomley wrote: > > > > I tried out the code at > > > > > > > > git://git.kernel.org/pub/scm/linux/kernel/git/viro/signal.git > > > > experimental-kernel_thread > > > > > > > > and it gives me this panic on boot. > > > > > > OK, found the fix: the idle thread is a kernel thread, but it doesn't > > > come through kernel_thread(). The fix is to check for it (fortunately > > > it has the signal usp == 0). > > > > Um... I see, but I really wonder if that's the right fix. FWIW, sparc > > will have the same problem... Hell knows. OTOH, it's a nice way to > > get of implicit interplay between copy_thread() and idle_regs() - note > > that SMP architectures doing default idle_regs() need to be damn careful > > about what they do in their "is that kernel thread" logics; all-zeros > > pt_regs might give varying results on user_mode(regs) tests, etc. > > Might be better to go for > > if (p->flags & PF_KTHREAD) { > > if (!usp) { > > we are starting an idle thread > > } else { > > we are setting things up for kernel_thread() > > } > > } else { > > we are forking > > } > > kind of logics, looking at regs only in the last case. And to hell with > > (separate and overridable) idle_regs() once everything goes that way... > > But there's not a lot of point. forking an idle thread actually doesn't > care about any of the register execution setup because it never really > uses it to execute. That's why it was safe for us to use the user > thread setup ... I suppose the interior of the kernel thread case could > be conditioned on if (usp). BTW, speaking of parisc copy_thread()... Why the hell do we bother with *cregs = *pregs in userland case? It's a part of task_struct, after all, and we have copied that wholesale in arch_dup_task_struct(). Another thing: why do we bother with STREG %r30,PT_GR21(%r1) in fork wrapper? We bloody well know what the offset will be, after all - right in the beginning of that sucker we'd done LDREG TI_TASK-THREAD_SZ_ALGN-FRAME_SIZE(%r30), %r1 so we rely on %r30 having been (unsigned long)current_thread_info() + THREAD_SZ_ALGN + FRAME_SIZE. Then we add FRAME_SIZE again. IOW, the offset is a known constant. Hell, in child_return you rely on its value... While we are at it, I'm not sure you need to go through wrapper_exit on the way out in parent - saving cr27 can be done via e.g. r28 instead of r3, at which point you can simply branch to sys_clone() with no work left for wrapper_exit. *Child* obviously needs to restore these registers, so let it do that in child_return, but why bother in parent? After all, we are talking about the callee-saved registers, so sys_clone() is going to revert whatever changes it makes to them... BTW, TIF_SYSCALL_TRACE and singlestepping are turned off in child, so I don't see any need for child_return to know where the parent had come from - it won't have anything to do in tracesys_exit anyway. I've folded your fixes and pushed the result; I've added (again, completely untested) optimizations along the lines of the above on top of those, as a separate commit. Comments? -- To unsubscribe from this list: send the line "unsubscribe linux-parisc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Oct 05, 2012 at 06:54:02PM -0400, John David Anglin wrote: > On 5-Oct-12, at 10:48 AM, Al Viro wrote: > > >What are you using for toolchain, BTW? With gcc 4.3 / binutils 2.20 > >(cross-builds) I'm getting panics from mainline kernel on parisc32 all > >way back to 2.6.28... > > > Current debian unstable tools are available in > shirka.esiee.fr:/home/dave/archives > and shirka.esiee.fr:/home/dave/archives-new. It's pretty straight > forward to update > binutils, gcc, libc, etc. > > gcc 4.4, 4.5, 4.6 and 4.7 are available. In particular, 4.6 and 4.7 > have significant > bug fixes. > > James seems to have an account on shirka. > > I don't have any place to distribute the debs that are on shirka, or > the time to remove > obsolete builds. Any volunteers? Hmm... debian-ports would be a natural place, but TBH I'd very much prefer to deal with cross-builds - doing kernel builds on a 100MHz 7100LC with 96M of RAM is really not fun... -- To unsubscribe from this list: send the line "unsubscribe linux-parisc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 5-Oct-12, at 7:32 PM, Al Viro wrote: > On Fri, Oct 05, 2012 at 06:54:02PM -0400, John David Anglin wrote: >> On 5-Oct-12, at 10:48 AM, Al Viro wrote: >> >>> What are you using for toolchain, BTW? With gcc 4.3 / binutils 2.20 >>> (cross-builds) I'm getting panics from mainline kernel on parisc32 >>> all >>> way back to 2.6.28... >> >> >> Current debian unstable tools are available in >> shirka.esiee.fr:/home/dave/archives >> and shirka.esiee.fr:/home/dave/archives-new. It's pretty straight >> forward to update >> binutils, gcc, libc, etc. >> >> gcc 4.4, 4.5, 4.6 and 4.7 are available. In particular, 4.6 and 4.7 >> have significant >> bug fixes. >> >> James seems to have an account on shirka. >> >> I don't have any place to distribute the debs that are on shirka, or >> the time to remove >> obsolete builds. Any volunteers? > > Hmm... debian-ports would be a natural place, but TBH I'd very much > prefer > to deal with cross-builds - doing kernel builds on a 100MHz 7100LC > with > 96M of RAM is really not fun... > Right... I had setup a buildd system system on lafayette.esiee.fr but unfortunately a processor module died. I also never received a response from debian- ports (several emails) about restarting buildd for parisc. I've continued to build packages on the rp3440 that I have at home. It doesn't have much trouble keeping up with unstable. At the same time, I use it for gcc testing and development. I would say you should be able to build cross tools using the binutils 2.22 branch and gcc 4.6 or 4.7. They should work better than gcc 4.3 with binutils 2.20. The reason I say that is I have built many debian packages with the current tools and I have been slowly chipping away at the toolchain bugs that I have found. Dave -- John David Anglin dave.anglin@bell.net -- To unsubscribe from this list: send the line "unsubscribe linux-parisc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/parisc/kernel/process.c b/arch/parisc/kernel/process.c index a67f122..44e8534 100644 --- a/arch/parisc/kernel/process.c +++ b/arch/parisc/kernel/process.c @@ -254,7 +254,7 @@ copy_thread(unsigned long clone_flags, unsigned long usp, extern void * const hpux_child_return; #endif - if (unlikely(p->flags & PF_KTHREAD)) { + if (unlikely((p->flags & PF_KTHREAD) && usp != 0)) { memset(cregs, 0, sizeof(struct pt_regs)); /* kernel thread */ cregs->ksp = (unsigned long)stack + THREAD_SZ_ALGN;