From owner-freebsd-hackers@FreeBSD.ORG Thu Mar 27 19:42:05 2014 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 07AAA6B8 for ; Thu, 27 Mar 2014 19:42:05 +0000 (UTC) Received: from elf.torek.net (50-73-42-1-utah.hfc.comcastbusiness.net [50.73.42.1]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id C9F72BFF for ; Thu, 27 Mar 2014 19:42:04 +0000 (UTC) Received: from elf.torek.net (localhost [127.0.0.1]) by elf.torek.net (8.14.5/8.14.5) with ESMTP id s2RJg2G6069599; Thu, 27 Mar 2014 13:42:02 -0600 (MDT) (envelope-from torek@elf.torek.net) Received: (from torek@localhost) by elf.torek.net (8.14.5/8.14.5/Submit) id s2RJg2hQ069598; Thu, 27 Mar 2014 13:42:02 -0600 (MDT) (envelope-from torek) Date: Thu, 27 Mar 2014 13:42:02 -0600 (MDT) From: Chris Torek Message-Id: <201403271942.s2RJg2hQ069598@elf.torek.net> To: kostikbel@gmail.com Subject: Re: slight problem with r254457 (kthread_add fix) In-Reply-To: <20140327120720.GN21331@kib.kiev.ua> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.4.3 (elf.torek.net [127.0.0.1]); Thu, 27 Mar 2014 13:42:02 -0600 (MDT) X-Mailman-Approved-At: Thu, 27 Mar 2014 20:48:04 +0000 Cc: freebsd-hackers@freebsd.org X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 27 Mar 2014 19:42:05 -0000 >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