From owner-svn-src-head@FreeBSD.ORG Tue Sep 2 15:41:35 2014 Return-Path: Delivered-To: svn-src-head@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 7E6E72FD; Tue, 2 Sep 2014 15:41:35 +0000 (UTC) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::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 02FF61A15; Tue, 2 Sep 2014 15:41:34 +0000 (UTC) Received: from tom.home (kib@localhost [127.0.0.1]) by kib.kiev.ua (8.14.9/8.14.9) with ESMTP id s82FfRJx003268 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 2 Sep 2014 18:41:27 +0300 (EEST) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.9.2 kib.kiev.ua s82FfRJx003268 Received: (from kostik@localhost) by tom.home (8.14.9/8.14.9/Submit) id s82FfRJI003267; Tue, 2 Sep 2014 18:41:27 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Tue, 2 Sep 2014 18:41:27 +0300 From: Konstantin Belousov To: John Baldwin Subject: Re: svn commit: r270850 - in head/sys: i386/i386 i386/include i386/isa x86/acpica Message-ID: <20140902154127.GD2737@kib.kiev.ua> References: <201408301748.s7UHmc6H059701@svn.freebsd.org> <20140830195809.GS2737@kib.kiev.ua> <201409021100.57493.jhb@freebsd.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="I2bJ1vO3Y/RENlXz" Content-Disposition: inline In-Reply-To: <201409021100.57493.jhb@freebsd.org> User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Status: No, score=-2.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FREEMAIL_FROM,NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.0 X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on tom.home Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 02 Sep 2014 15:41:35 -0000 --I2bJ1vO3Y/RENlXz Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Sep 02, 2014 at 11:00:57AM -0400, John Baldwin wrote: > On Saturday, August 30, 2014 3:58:09 pm Konstantin Belousov wrote: > > On Sat, Aug 30, 2014 at 05:48:38PM +0000, John Baldwin wrote: > > > Author: jhb > > > Date: Sat Aug 30 17:48:38 2014 > > > New Revision: 270850 > > > URL: http://svnweb.freebsd.org/changeset/base/270850 > > >=20 > > > Log: > > > Save and restore FPU state across suspend and resume. In earlier= =20 > revisions > > > of this patch, resumectx() called npxresume() directly, but that do= esn't > > > work because resumectx() runs with a non-standard %cs selector. =20 > Instead, > > > all of the FPU suspend/resume handling is done in C. > > > =20 > > > MFC after: 1 week > > >=20 > > > Modified: > > > head/sys/i386/i386/mp_machdep.c > > > head/sys/i386/i386/swtch.s > > > head/sys/i386/include/npx.h > > > head/sys/i386/include/pcb.h > > > head/sys/i386/isa/npx.c > > > head/sys/x86/acpica/acpi_wakeup.c > > >=20 > > > Modified: head/sys/i386/i386/mp_machdep.c > > >=20 > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D > > > --- head/sys/i386/i386/mp_machdep.c Sat Aug 30 17:39:28 2014 (r270849) > > > +++ head/sys/i386/i386/mp_machdep.c Sat Aug 30 17:48:38 2014 (r270850) > > > @@ -1522,9 +1522,15 @@ cpususpend_handler(void) > > > =20 > > > cpu =3D PCPU_GET(cpuid); > > > if (savectx(susppcbs[cpu])) { > > > +#ifdef DEV_NPX > > > + npxsuspend(&suspcbs[cpu]->pcb_fpususpend); > > > +#endif > > > wbinvd(); > > > CPU_SET_ATOMIC(cpu, &suspended_cpus); > > > } else { > > > +#ifdef DEV_NPX > > > + npxresume(&suspcbs[cpu]->pcb_fpususpend); > > > +#endif > > > pmap_init_pat(); > > > PCPU_SET(switchtime, 0); > > > PCPU_SET(switchticks, ticks); > > >=20 > > > Modified: head/sys/i386/i386/swtch.s > > >=20 > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D > > > --- head/sys/i386/i386/swtch.s Sat Aug 30 17:39:28 2014 (r270849) > > > +++ head/sys/i386/i386/swtch.s Sat Aug 30 17:48:38 2014 (r270850) > > > @@ -416,45 +416,6 @@ ENTRY(savectx) > > > sldt PCB_LDT(%ecx) > > > str PCB_TR(%ecx) > > > =20 > > > -#ifdef DEV_NPX > > > - /* > > > - * If fpcurthread =3D=3D NULL, then the npx h/w state is irrelevant= and=20 > the > > > - * state had better already be in the pcb. This is true for forks > > > - * but not for dumps (the old book-keeping with FP flags in the pcb > > > - * always lost for dumps because the dump pcb has 0 flags). > > > - * > > > - * If fpcurthread !=3D NULL, then we have to save the npx h/w state= to > > > - * fpcurthread's pcb and copy it to the requested pcb, or save to t= he > > > - * requested pcb and reload. Copying is easier because we would > > > - * have to handle h/w bugs for reloading. We used to lose the > > > - * parent's npx state for forks by forgetting to reload. > > > - */ > > > - pushfl > > > - CLI > > > - movl PCPU(FPCURTHREAD),%eax > > > - testl %eax,%eax > > > - je 1f > > > - > > > - pushl %ecx > > > - movl TD_PCB(%eax),%eax > > > - movl PCB_SAVEFPU(%eax),%eax > > > - pushl %eax > > > - pushl %eax > > > - call npxsave > > > - addl $4,%esp > > > - popl %eax > > > - popl %ecx > > > - > > > - pushl $PCB_SAVEFPU_SIZE > > > - leal PCB_USERFPU(%ecx),%ecx > > > - pushl %ecx > > > - pushl %eax > > > - call bcopy > > > - addl $12,%esp > > > -1: > > > - popfl > > > -#endif /* DEV_NPX */ > > > - > > > movl $1,%eax > > > ret > > > END(savectx) > > > @@ -519,10 +480,6 @@ ENTRY(resumectx) > > > movl PCB_DR7(%ecx),%eax > > > movl %eax,%dr7 > > > =20 > > > -#ifdef DEV_NPX > > > - /* XXX FIX ME */ > > > -#endif > > > - > > > /* Restore other registers */ > > > movl PCB_EDI(%ecx),%edi > > > movl PCB_ESI(%ecx),%esi > > >=20 > > > Modified: head/sys/i386/include/npx.h > > >=20 > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D > > > --- head/sys/i386/include/npx.h Sat Aug 30 17:39:28 2014 (r270849) > > > +++ head/sys/i386/include/npx.h Sat Aug 30 17:48:38 2014 (r270850) > > > @@ -53,8 +53,10 @@ void npxexit(struct thread *td); > > > int npxformat(void); > > > int npxgetregs(struct thread *td); > > > void npxinit(void); > > > +void npxresume(union savefpu *addr); > > > void npxsave(union savefpu *addr); > > > void npxsetregs(struct thread *td, union savefpu *addr); > > > +void npxsuspend(union savefpu *addr); > > > int npxtrap_x87(void); > > > int npxtrap_sse(void); > > > void npxuserinited(struct thread *); > > >=20 > > > Modified: head/sys/i386/include/pcb.h > > >=20 > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D > > > --- head/sys/i386/include/pcb.h Sat Aug 30 17:39:28 2014 (r270849) > > > +++ head/sys/i386/include/pcb.h Sat Aug 30 17:48:38 2014 (r270850) > > > @@ -90,6 +90,8 @@ struct pcb { > > > struct region_descriptor pcb_idt; > > > uint16_t pcb_ldt; > > > uint16_t pcb_tr; > > > + > > > + union savefpu pcb_fpususpend; > > > }; > > Now pcb consumes 512 bytes from each thread' kernel stack, which mostly > > stay unused. > >=20 > > Amd64 only stores the pointer to the fpususpend context in pcb, and > > acpu_wakeup() allocates the memory as needed. Even this is a waste of 8 > > bytes which are not needed for normal kernel operations. > >=20 > > Suspend FPU context, as well as amd64 MSRs should go out of pcb into > > some per-cpu suspend data block. >=20 > I thought about that. I could easily make a parallel array, or perhaps u= se a=20 > separate 'susppcb' structure that includes a pcb and the savefpu union and > change susppcbs to be an array of those. Which do you prefer? If we want > to move some state out of the PCB on amd64 into this, then a separate str= uct > for susppcbs might be the sanest. Yes, separate structure seems to be a way forward. FWIW, I do not understand the need for pcb in its current form, allocated on the thread kernel stack, at all. It looks like a vestige of the u-area, but serves no real purpose except to consume now precious stack space. The idea of the part of the thread state that can be swapped out, together with the stack, seems to become alien. Most of the thread state which is not needed when the thread is not runnable, now goes to struct thread anyway. Might be, we should move the pcb into td_md. People actively object to the idea of the swappable kernel stack when they learn hard that local vars cannot participate in the global lists. The only thing which is currently allocated below the pcb and which seems to be reasonable to swap out, is the FPU context on amd64. --I2bJ1vO3Y/RENlXz Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJUBeUmAAoJEJDCuSvBvK1BRY8P/jJrYZlapubg7Gq/IcOlHD30 j8wk2GUR09kK+97FlpZ84SFcuQTNjguI9ygaDXbjYxmEcEGcV2WJFFIBA5MPpi2R D4MCYtHwdYHAdQ3umKBRWi/Jnx1Ctpq69pDYlzbMd39EnBfNR9MJbN5AHJ95Z7Kq Hm9TWYVIXIvi6eTcwz4q9agbbcn6S3UqUGQYbOQ5u8fUcbuzxhmxx8j/d9F8xg1H 3myJ1N8A2gf5b5Ux2+bLlE8+V5SsHutNj/2fcIVEppSAVBNxlhYBkZKcuZw9TgQL y4F73p3y53ppfaK+cyClwbnV35qH5MxOgWqAEusVr2DvLMNfQbWOnkUkXTG4pKIL BPPaaJAOUqEbdU+fE5EbKSoHKPY5fD+gYD70Eyml5G6H72HzJYvzLgpfIj4M2s0X v9MWAJvdo1aHRuu2D54A00fsmBUp9CVKp2cyfOZChJTJhnlTTcP7PgFCoDNG6MKt Q7ukiXjzl+B+c0aeqxkn4vy7RTR5K4drkbyqlExg7lrYjhryrt+k/LWJALIMVQnX qMjylHOvtQ0vxroZb0Goh8Mreovr7fKtxJfQsyaAiBTU8H3pKsQNm7F70Eq6LYBR wncIIpiHdM+xLNpo+bPA2SYpsxNOAGkqJ4s1+H0kpxf4bFQc8bm8dmofsVwoJQeg EhOErA1bkAZE7SZbfvM3 =fmsT -----END PGP SIGNATURE----- --I2bJ1vO3Y/RENlXz--