From owner-freebsd-hackers@FreeBSD.ORG Fri Mar 28 07:03:06 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 71213AE3; Fri, 28 Mar 2014 07:03:06 +0000 (UTC) Received: from vps1.elischer.org (vps1.elischer.org [204.109.63.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "vps1.elischer.org", Issuer "CA Cert Signing Authority" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 3B75039F; Fri, 28 Mar 2014 07:03:06 +0000 (UTC) Received: from julian-mbp3.pixel8networks.com ([12.179.176.146]) (authenticated bits=0) by vps1.elischer.org (8.14.8/8.14.8) with ESMTP id s2S72vCR020812 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Fri, 28 Mar 2014 00:02:58 -0700 (PDT) (envelope-from julian@freebsd.org) Message-ID: <53351E96.1000608@freebsd.org> Date: Fri, 28 Mar 2014 00:02:46 -0700 From: Julian Elischer User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:24.0) Gecko/20100101 Thunderbird/24.4.0 MIME-Version: 1.0 To: freebsd-hackers@freebsd.org, Chris Torek , Konstantin Belousov Subject: Re: slight problem with r254457 (kthread_add fix) References: <201403271942.s2RJg2hQ069598@elf.torek.net> In-Reply-To: <201403271942.s2RJg2hQ069598@elf.torek.net> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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: Fri, 28 Mar 2014 07:03:06 -0000 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" >