Date: Fri, 28 Mar 2014 16:24:02 +0200 From: Konstantin Belousov <kostikbel@gmail.com> To: Chris Torek <torek@mail.torek.net> Cc: freebsd-hackers@freebsd.org Subject: Re: slight problem with r254457 (kthread_add fix) Message-ID: <20140328142402.GX21331@kib.kiev.ua> In-Reply-To: <201403271942.s2RJg2hQ069598@elf.torek.net> References: <20140327120720.GN21331@kib.kiev.ua> <201403271942.s2RJg2hQ069598@elf.torek.net>
next in thread | previous in thread | raw e-mail | index | archive | help
--BzhqTaE66jeIVBY0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Mar 27, 2014 at 01:42:02PM -0600, 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 ? >=20 > 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.) The kernel-FPU code only exists for x86. >=20 > >If yes, then the following patch should handle the problem, hopefully. >=20 > I'm OK with this too, as long as all architectures do it, or > something sufficiently similar. Did you tested the patch I posted, for your situation ? >=20 > (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. >=20 > 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.) It seems that you formulation somewhat contradictory. The cpu_throw is used only to do the last switch out of the exiting thread. And indeed, I think that there is a bug, on x86. The CR0.TS must be cleared, and fpcurthread must be reset if current cpu still carries FPU state of the curthread. At least, I do not see anything which would guarantee that CLTS was done before cpu_throw. --BzhqTaE66jeIVBY0 Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (FreeBSD) iQIcBAEBAgAGBQJTNYYBAAoJEJDCuSvBvK1BQRQP/3k+mI3nybXeu5cXbe9Bm6DU tDvxt5YYJXnlgcIbK1pPTqeso40acoOzBeIFG2mGZ8oz6Oiho1asAU9ZT2cnQlsx kFEKiaTCMS12voOExBsjZGx5ygsLcexBpe+lVUpKBpj4+hpbg8BoEojCnInEh5CI Bz/IqVWLE5g6l7x4ejNYsR/Q4qQlopoLjMVHi2RqmSoVFH72DDaCyBxz1WKp2OlG daTh1redgGwN4f+QctN2Ikgt7cMfgicLw94DwI7T+f0qMajTBzSbkB2RrLuobKtB r9/Os+Gp5Br8PMyLzPdoQQOniSTCNW+6Q+Ot/C4w/fgfEfGC9Z0q0L7+gnX/ezTy 65pCPjhiRBZVl2zqyM13+1/JpNHcLJuuRqJb9aHvOGbsmoiC2WZixt/f7U1pnwxV I/RUPyllo8QH9KrhIhh29tVyeaS3/FK1ZLeFS1NCWes05jJdigGNg6fwmdbRMp4i AApAjWku2n7FOpULRtmL8KlmYQWpcrOA4EDMW2f85HeIfP1vQVeAmlmJ5cP+Jy+Z mb9hF++OGuqDIvweUWFO1hIzyf9P6kb7hw+ml6cMCHKo9MjCZC/MK6bF0GSabhwn gEHdPTtSjB2zb009dij61Y5MYRTM0hUeZ6Z5MrCMgldzaLidh50PNdFWeoST89nd yzwyHYWE3uPil8F4bkrm =K6M3 -----END PGP SIGNATURE----- --BzhqTaE66jeIVBY0--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140328142402.GX21331>