Date: Fri, 28 Mar 2014 00:02:46 -0700 From: Julian Elischer <julian@freebsd.org> To: freebsd-hackers@freebsd.org, Chris Torek <torek@torek.net>, Konstantin Belousov <kib@FreeBSD.org> Subject: Re: slight problem with r254457 (kthread_add fix) Message-ID: <53351E96.1000608@freebsd.org> In-Reply-To: <201403271942.s2RJg2hQ069598@elf.torek.net> References: <201403271942.s2RJg2hQ069598@elf.torek.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On 3/27/14, 12:42 PM, Chris Torek wrote: >> I agree with the problem statement, but disagree with the proposed >> 'fix'. I.e., I agree that on the creation of a new kernel thread, the >> current thread FPU grab state must not leak into the created thread. >> >> In fact, cpu_set_upcall() already almost handles this correctly, by >> resetting the pcb_save pointer back to the user save area. What is >> not done is the clearing of PCB_KERNFPU flag, which seems to be the >> issue you met. Am I right ? > Yes. I considered your fix as well but decided it required also > understanding other architectures too much. :-) (I don't know > if sparc has similar code, for instance, but it *could*; its > FPU is much like the amd64 one. You've fixed i386, but there > are more.) > >> If yes, then the following patch should handle the problem, hopefully. > I'm OK with this too, as long as all architectures do it, or > something sufficiently similar. this is the reason that I originally made the new thread 'fork' off an old one (back in 2000 I think it was.. I was in Budapest I remember.), anyway I didn't want to be bothered with the internal contents of stuff I didn't understand on processors I didn't know, and the rule was "no FPU in the Kernel", so it was "safe" (ish). I did make a mental note that if there were ever problems with this then we should write a machine dependent function to set up a new thread context, but I was in a hurry so I did it in the most generic way possible. In any case, fork() had been doing it that way for years too. SO, I guess the message is: "no magic was hidden here. any obvious fix you can see is probably good enough" BTW for kernel threads with no proc, My memory is that I was forking from thread0, which was the thread that went on to be running init(). I THINK that is still true but either way, it was not a thread that was going anywhere, and it was guaranteed to not have a floating point operation. > (Another way to look at this is that creating a new kernel thread > requires cloning a child from somewhere, but we don't have a > particularly good "somewhere" so we clone something chosen at what > amounts to random. We then have to make this look like a "virgin > birth", untainted by any previous actions of the thing we're > cloning. > > While I was hunting this problem I also looked at cpu_throw in > amd64/amd64/cpu_switch.S. It took me a long time to figure out > why it is safe to ignore the per-cpu fpcurthread there. It is, > because cpu_throw is used for only one case, the "virgin birth" > setup on each CPU. There can be no existing thread using the FPU > at that point, so this is OK.) > > Chris > _______________________________________________ > freebsd-hackers@freebsd.org mailing list > http://lists.freebsd.org/mailman/listinfo/freebsd-hackers > To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@freebsd.org" >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?53351E96.1000608>