From owner-cvs-all@FreeBSD.ORG Sun May 28 06:44:41 2006 Return-Path: X-Original-To: cvs-all@freebsd.org Delivered-To: cvs-all@freebsd.org Received: from localhost.my.domain (localhost [127.0.0.1]) by hub.freebsd.org (Postfix) with ESMTP id 5DEEC16B15F; Sun, 28 May 2006 06:38:11 +0000 (UTC) (envelope-from davidxu@freebsd.org) From: David Xu To: Bruce Evans Date: Sun, 28 May 2006 14:38:06 +0800 User-Agent: KMail/1.8.2 References: <200605280203.k4S23DfP053792@repoman.freebsd.org> <20060528134646.V19811@delplex.bde.org> In-Reply-To: <20060528134646.V19811@delplex.bde.org> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200605281438.06966.davidxu@freebsd.org> Cc: cvs-src@freebsd.org, src-committers@freebsd.org, cvs-all@freebsd.org Subject: Re: cvs commit: src/sys/i386/include npx.h src/sys/i386/i386 vm_machdep.c src/sys/i386/isa npx.c X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 28 May 2006 06:44:53 -0000 On Sunday 28 May 2006 13:15, Bruce Evans wrote: > On Sun, 28 May 2006, David Xu wrote: > > davidxu 2006-05-28 02:03:13 UTC > > > > FreeBSD src repository > > > > Modified files: > > sys/i386/include npx.h > > sys/i386/i386 vm_machdep.c > > sys/i386/isa npx.c > > Log: > > When creating a new thread, inherit floating-point environment from > > current thread, this is required by POSIX pthread_create document. > > > > Revision Changes Path > > 1.272 +1 -0 src/sys/i386/i386/vm_machdep.c > > 1.30 +4 -0 src/sys/i386/include/npx.h > > 1.167 +30 -0 src/sys/i386/isa/npx.c > > POSIX seems to require inheriting the entire environment, but this > change does extra work to disinherit everything except the control > word(s), except in the fxsr case it is missing the extra work to > disinherit the status bits in the mxcsr. No, pthread_create is not fork, they are different. http://www.opengroup.org/onlinepubs/009695399/basedefs/fenv.h.html I think you misunderstood x86's float-pointing status and POSIX float point environment. POSIX defined following environment flags: FE_DOWNWARD FE_TONEAREST FE_TOWARDZERO FE_UPWARD which when implemented by x86, they are in control word, status word is nothing to do with new threads, no pending exceptions should be inherited by new thread. > BTW, setjmp()/longjmp() has > much larger bugs in this area (they preserve the FP control word but > don't even know about mxcsr even on amd64 where mxcsr is primary, so > they they mess up both the control and status in mxcsr). > > This change also does a lot of work to determine the control word(s), > and a lot of work to use the old control word(s) even if the old control > word(s) are the default. This was already fixed in my second commit. > The case where the old thread has the default > (!PCB_NPXINITDONE) FP state should be optimized by doing nothing except > checking for this case. Otherwise, if preserving only the control > word(s) is required, some optimizations are possible with extra > complications (*). Otherwise (when the whole environment needs to be > preserved then it is simplest and fastest to copy the entire state > (the data registers may need to be cleared for security, but not doing > so would give less important leaks than already exist). Then the new > interface wouldn't be needed. > > (*) The control word(s) can be obtained much more efficiently than using > npxgetregs(). In most cases the control word(s) won't have changed from > their default and nothing needs to be done except for checking that they > haven't changed. OTOH, the FP environment contains status words which > have changed in most cases, so this sort of optimization won't work if > the FP environment must be inherited. > Well, I am trying to do things correctly, fix bugs and then optimize it, rather than do nothing, and leave the bug there. > This change has some style bugs (with highest density in npx.h). > > Bruce