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

Re: [Xen-devel] [PATCH v3 04/15] x86: refactor psr: Encapsulate 'cbm_len' and 'cbm_max'



>>> On 30.11.16 at 10:08, <yi.y.sun@xxxxxxxxxxxxxxx> wrote:
> On 16-11-29 02:43:24, Jan Beulich wrote:
>> >>> On 29.11.16 at 05:38, <yi.y.sun@xxxxxxxxxxxxxxx> wrote:
>> > To make codes be better reviewable, I propose below three suggestions:
>> > 1) I write a design document about refactor to make you more easily 
>> > understand the ideas.
>> 
>> This is appreciated, but I don't think it's strictly necessary. Describing
>> the new design (rather than the changes to the existing one) is what
>> likely would be more useful (I'm sorry if I've misunderstood what you
>> said, and you in fact had meant just this), which iirc you already have
>> in patch 1.
>> 
>> > 2) Replace the psr.c with a new file which discards all old codes so
>> > that you do not need to consider old implementations which may cause
>> > confusion.
>> > 3) Discard the refactor codes. Just implement L2 CAT based on current
>> > codes. Because L2 CAT does not have much difference with L3.
>> 
>> I don't think introducing a new file is the ideal approach. I'd suggest
>> to rip out the entire implementation in a first patch, leaving just
>> empty functions to avoid breaking the build (i.e. perhaps mostly the
>> ones used by domctl/sysctl, and maybe some init one). Then
>> introduce new code, ideally of course not in one big patch, but
>> broken up into logical pieces where possible (one such split would be
>> that of course you don't need to re-implement domctl/sysctl handling
>> in the same patch as everything else).
>> 
> Thanks for your suggestion! Just want to confirm if my understanding is
> right. So, you mean I can remove all old codes but keep the interfaces as
> empty functions to make sure the build can pass. Then, implement whole
> functionality step by step. Right?

Yes.

Thanks, 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®.