On Thu, Sep 04, 2014 at 10:50:25PM -0400, John Baldwin wrote: > On Tuesday, September 02, 2014 06:41:27 PM Konstantin Belousov wrote: > > On Tue, Sep 02, 2014 at 11:00:57AM -0400, John Baldwin wrote: > > > I thought about that. I could easily make a parallel array, or perhaps > > > use a 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 struct for susppcbs might be the sanest. > > > > Yes, separate structure seems to be a way forward. > > Please see www.freebsd.org/~jhb/patches/susppcb.patch Note that I moved > fpususpend() out into a C function on amd64 so that resumectx() could still > operate on just a pcb. This also makes savectx and resumectx more symmetric > and matches what I ended up doing on i386. This is tested for suspend and > resume on both i386 and amd64.
The implementation of fpuresume() in C is definitely an improvement. You only moved the fpu context to the susppcb, I think this is good for now, we will want to move other bits later. Do we need to keep pcb layout for KBI compat ? I remember that pcb size is asserted to properly fit into pcpu area for percpu zones. But why keep the layout ? I.e. moving all padding bits to the end. There is one weird detail, not touched by your patch. Amd64 resume path calls initializecpu(), while i386 does not. I do not see any use for the call, the reload of CRX registers by trampoline/resumectx should already set everything to working state. E.g., enabling XMM in CR4 after fpu state is restored looks strange. Overall, it looks fine. Do you prefer to have alloc_fpusave() on i386 ?
pgpfGg5ZlcuvK.pgp
Description: PGP signature
