Re: [Xen-devel] [PATCH v4 02/24] x86: refactor psr: remove L3 CAT/CDP codes.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 14.12.16 at 05:07, <yi.y.sun@xxxxxxxxxxxxxxx> wrote:
> The current cache allocation codes in psr.c do not consider
> future features addition and are not friendly to extend.
> 
> To make psr.c be more flexible to add new features and fulfill
> the program principle, open for extension but closed for
> modification, we have to refactor the psr.c:
> 1. Analyze cache allocation features and abstract general data
>    structures.
> 2. Analyze the init and all other functions flow, abstract all
>    steps that different features may have different implementations.
>    Make these steps be callback functions and register feature
>    specific fuctions. Then, the main processes will not be changed
>    when introducing a new feature.
> 
> Because the quantity of refactor codes is big and the logics are
> changed a lot, it will cause reviewers confused if just change
> old codes. Reviewers have to understand both old codes and new
> implementations. After review iterations from V1 to V3, Jan has
> proposed to remove all old cache allocation codes firstly, then
> implement new codes step by step. This will help to make codes
> be more easily reviewable.
> 
> There is no construction without destruction. So, this patch
> removes all current L3 CAT/CDP codes in psr.c. The following
> patches will introduce the new mechanism.
> 
> Signed-off-by: Yi Sun <yi.y.sun@xxxxxxxxxxxxxxx>

Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
with one question:

> @@ -738,14 +282,10 @@ static int __init psr_presmp_init(void)
>      if ( (opt_psr & PSR_CMT) && opt_rmid_max )
>          init_psr_cmt(opt_rmid_max);
>  
> -    if ( opt_psr & PSR_CAT )
> -        init_psr_cat();
> -
> -    if ( psr_cpu_prepare(0) )
> -        psr_cat_free();
> +    psr_cpu_prepare(0);
>  
>      psr_cpu_init();
> -    if ( psr_cmt_enabled() || cat_socket_info )
> +    if ( psr_cmt_enabled() )
>          register_cpu_notifier(&cpu_nfb);

You retain this notifier - do you expect to need it going forward?
Till now it was used to allocate that strange temporary object,
which iirc we've discussed (during the earlier versions) would go
away.

Jan


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