WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

RE: [Xen-devel] [PATCH][RFC] FPU LWP 1/5: clean up the FPU code

To: Keir Fraser <keir@xxxxxxx>, "'xen-devel@xxxxxxxxxxxxxxxxxxx'" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: RE: [Xen-devel] [PATCH][RFC] FPU LWP 1/5: clean up the FPU code
From: "Huang2, Wei" <Wei.Huang2@xxxxxxx>
Date: Fri, 15 Apr 2011 11:22:52 -0500
Accept-language: en-US
Acceptlanguage: en-US
Cc:
Delivery-date: Fri, 15 Apr 2011 09:27:44 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <C9CDCC5D.2CB27%keir@xxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <4DA75B22.1010702@xxxxxxx> <C9CDCC5D.2CB27%keir@xxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: Acv7TlMT8R7R1vb5gEuLbgGsms8OnQAORNQg
Thread-topic: [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

<Prev in Thread] Current Thread [Next in Thread>