[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [v2 1/3] x86: refactor psr implementation in hypervisor.



>>> On 09.10.16 at 08:43, <yi.y.sun@xxxxxxxxxxxxxxx> wrote:
> On 16-09-30 17:18:33, Konrad Rzeszutek Wilk wrote:
>> On Thu, Sep 22, 2016 at 10:15:20AM +0800, Yi Sun wrote:
>> > Current psr.c is designed for supporting L3 CAT/CDP. It has many
>> > limitations to add new feature. Considering to support more PSR
>> > features, we need refactor PSR implementation to make it more
>> > flexible and fulfill the principle, open for extension but closed
>> > for modification.
>> > 
>> > The core of the refactoring is to abstract the common actions and
>> > encapsulate them into "struct feat_ops". The detailed steps to add
>> > a new feature are described at the head of psr.c.
>> > 
>> > Signed-off-by: Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx>
>> > 
>> > ---
>> > Changed since v1:
>> >  * sysctl.c
>> >     - Interface change for abstraction requirement.
>> >  * psr.c
>> >     - Function and variables names are changed to express accurately.
>> >     - Fix code style issues.
>> >     - Fix imprecise comments.
>> >     - Add one callback function, get_cos_num(), to fulfill the
>> >       abstraction requirement.
>> >     - Divide get_old_set_new() callback functions into two functions:
>> >       get_old_val() and set_new_val() make it more primitive.
>> >     - Change feat_info type from an array to union.
>> >     - Adjust some functions to replace if to switch to make them
>> >       clearer.
>> >     - Replace custom list management to system.
>> >     - Use 'const' to make codes more safe.
>> >  * psr.h
>> >     - Change 'enum mask_type' to 'enum psr_val_type' to express
>> >       more accurate.
>> >     - Change parameters of psr_get_info() to fulfill abstraction
>> >       requirement.
>> > ---
>> >  xen/arch/x86/domctl.c     |   36 +-
>> >  xen/arch/x86/psr.c        | 1105 
>> > +++++++++++++++++++++++++++++++++++----------
>> 
>> Whoa. 1K changes. This is a dense patch.
>> 
> Yes, a little big because it refactors psr.c. :-)

Please nevertheless strive to break it up some, to aid reviewing. I
have to admit that I'm on the edge of NAK-ing such a hard to review
change to code that is of no core interest (at least as long as all of
this remains Atom-only).

Also to both of you: Please limit the quoting in your replies.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.