Date: Thu, 27 Mar 2014 13:42:02 -0600 (MDT) From: Chris Torek <torek@elf.torek.net> To: kostikbel@gmail.com Cc: freebsd-hackers@freebsd.org Subject: Re: slight problem with r254457 (kthread_add fix) Message-ID: <201403271942.s2RJg2hQ069598@elf.torek.net> In-Reply-To: <20140327120720.GN21331@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
>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. (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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201403271942.s2RJg2hQ069598>