Re: [Xen-devel] [PATCH v3 02/15] x86: refactor psr: Split 'ref' out.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.10.16 at 05:40, <yi.y.sun@xxxxxxxxxxxxxxx> wrote:
> 'ref' in 'struct psr_cat_cbm' is used as a reference count for
> one COS register. It doesn't relate to cbm. So, it can be splitted
> out.

split

> --- a/xen/arch/x86/psr.c
> +++ b/xen/arch/x86/psr.c
> @@ -23,6 +23,9 @@
>  #define PSR_CAT        (1<<1)
>  #define PSR_CDP        (1<<2)
>  
> +/* Per spec, the maximum COS register number is 128. */
> +#define MAX_COS_REG_NUM  128

Is this the maximum register number or the maximum possible
count of registers? If the latter (which I assume base on how it
gets used below), please prefer COUNT or CNT over NUM.

> @@ -31,14 +34,19 @@ struct psr_cat_cbm {
>              uint64_t data;
>          };
>      };
> -    unsigned int ref;
>  };
>  
>  struct psr_cat_socket_info {
>      unsigned int cbm_len;
>      unsigned int cos_max;
>      struct psr_cat_cbm *cos_to_cbm;
> -    spinlock_t cbm_lock;
> +    /*
> +     * Every entry of cos_ref is the reference count of a COS register.
> +     * One entry of cos_ref corresponds to one COS ID.
> +     */
> +    unsigned int cos_ref[MAX_COS_REG_NUM];
> +    /* Protect cos_ref */
> +    spinlock_t ref_lock;

So till now we had opt_cos_max + 1 refs per socket. Now all of the
sudden we get 128 of them. Is that really needed? Shouldn't this be
a pointer, and space allocated just like for info->cos_to_cbm in
cat_cpu_prepare()?

> @@ -54,7 +62,7 @@ static unsigned long *__read_mostly cdp_socket_enable;
>  
>  static unsigned int opt_psr;
>  static unsigned int __initdata opt_rmid_max = 255;
> -static unsigned int __read_mostly opt_cos_max = 255;
> +static unsigned int __read_mostly opt_cos_max = MAX_COS_REG_NUM - 1;

This change tells me that th previous author thought the specified
maximum is higher. Would you please point me to where this new
lower limit is written down?

> @@ -574,7 +585,7 @@ static int cat_cpu_prepare(unsigned int cpu)
>  
>      if ( temp_cos_to_cbm == NULL &&
>           (temp_cos_to_cbm = xzalloc_array(struct psr_cat_cbm,
> -                                          opt_cos_max + 1UL)) == NULL )
> +                                          MAX_COS_REG_NUM)) == NULL )

Why?

Jan


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