[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] RE: [Xen-devel] [PATCH][RFC] FPU LWP 1/5: clean up the FPU code
Hi Keir, Following your comments, I will separate out xsave out of FPU code and put them into xstate.[ch]. The function will be renamed consistently. Thanks, -Wei -----Original Message----- From: Keir Fraser [mailto:keir.xen@xxxxxxxxx] On Behalf Of Keir Fraser Sent: Friday, April 15, 2011 4:20 AM To: Huang2, Wei; 'xen-devel@xxxxxxxxxxxxxxxxxxx' Subject: Re: [Xen-devel] [PATCH][RFC] FPU LWP 1/5: clean up the FPU code On 14/04/2011 21:37, "Wei Huang" <wei.huang2@xxxxxxx> wrote: > This patch reorganizes and cleans up the FPU code. Main changes include: > > 1. Related functions are re-arranged together. > 2. FPU save/restore code are extracted out as independent functions > (fsave/frstor, fxsave/fxrstor, xsave/xrstor). > 3. Two FPU entry functions, fpu_save() and fpu_restore(), are defined. > They utilize xsave/xrstor to save/restore FPU state if CPU supports > extended state. Otherwise they fall back to fxsave/fxrstor or fsave/frstor. Wei, If you're going to clean this up, please don't politely skirt around the existing file- and function-name conventions. The fact is that i387/fpu is but one item of state handled by XSAVE/XRSTOR and friends. Thus it is the tail wagging the dog to keep all the new mechanism in files and functions called {i387,fpu}*. I'd prefer you to move some or all functionality into newly-named files and functions -- perhaps using a prefix like xstate_ on functions and sticking it all in files xstate.[ch]. Perhaps the old i387 files would disappear completely, or perhaps you'll find a few bits and pieces logically remain in them, I don't mind either way as long as everything is in a logical place with a logical name. The naming in your 3rd patch is also unnecessarily confusing: is it really clear the difference between *_reload and *_restore, for example, that one is done on context switch and the other on #NM? We need better names; for example: xstate_save: Straightforward I guess as we always unlazily save everything that's dirty straight away during context switch. xstate_restore_lazy: Handle stuff we restore lazily on #NM. xstate_restore_eager: Handle stuff we restore unconditionally (if in use by the guest) on ctxt switch. These names would mean a lot more to me than what you chose. However, feel free to work out a comprehensive naming scheme that you think works best. All I'm saying is that our current naming scheme is already pretty crappy, and you make it quite a bit worse by following the existing direction way too much! -- Keir > Signed-off-by: Wei Huang <wei.huang2@xxxxxxx> > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@xxxxxxxxxxxxxxxxxxx > http://lists.xensource.com/xen-devel _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |