Re: [Xen-devel] [PATCH v4 05/12] x86: maintain socket CPU mask for CAT

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 09/04/2015 10:18, Chao Peng wrote:
> Some CAT resource/registers exist in socket level and they must be
> accessed from the CPU of the corresponding socket. It's common to pick
> an arbitrary CPU from the socket. To make the picking easy, it's useful
> to maintain a reference to the cpu_core_mask which contains all the
> siblings of a CPU in the same socket. The reference needs to be
> synchronized with the CPU up/down.
>
> Signed-off-by: Chao Peng <chao.p.peng@xxxxxxxxxxxxxxx>
> ---
>  xen/arch/x86/psr.c | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
>
> diff --git a/xen/arch/x86/psr.c b/xen/arch/x86/psr.c
> index 4aff5f6..7de2504 100644
> --- a/xen/arch/x86/psr.c
> +++ b/xen/arch/x86/psr.c
> @@ -32,6 +32,7 @@ struct psr_cat_socket_info {
>      unsigned int cbm_len;
>      unsigned int cos_max;
>      struct psr_cat_cbm *cos_cbm_map;
> +    cpumask_t *socket_cpu_mask;

At a pinch, you could get away with just "cpus" as a variable name, as
it is part of a structure, and instanced in an array, with "socket" in
the name.

>  };
>  
>  struct psr_assoc {
> @@ -234,6 +235,8 @@ static void cat_cpu_init(unsigned int cpu)
>      ASSERT(socket < nr_sockets);
>  
>      info = cat_socket_info + socket;
> +    if ( info->socket_cpu_mask == NULL )
> +        info->socket_cpu_mask = per_cpu(cpu_core_mask, cpu);

Surely after the test_and_set_bool() ?

>  
>      /* Avoid initializing more than one times for the same socket. */
>      if ( test_and_set_bool(info->initialized) )
> @@ -274,6 +277,24 @@ static void psr_cpu_init(unsigned int cpu)
>      psr_assoc_init(cpu);
>  }
>  
> +static void psr_cpu_fini(unsigned int cpu)

cat_cpu_fini() to mirror cat_cpu_init() or perhaps both?

> +{
> +    unsigned int socket, next;
> +    cpumask_t *cpu_mask;
> +
> +    if ( cat_socket_info )
> +    {
> +        socket = cpu_to_socket(cpu);
> +        cpu_mask = cat_socket_info[socket].socket_cpu_mask;
> +
> +        if ( (next = cpumask_cycle(cpu, cpu_mask)) == cpu )
> +            cat_socket_info[socket].socket_cpu_mask = NULL;
> +        else
> +            cat_socket_info[socket].socket_cpu_mask =
> +                                    per_cpu(cpu_core_mask, next);

Might it be easier to copy cpu_core_mask rather than playing these games
to avoid pointing into a stale per_cpu() area?

~Andrew

> +    }
> +}
> +
>  static int cpu_callback(
>      struct notifier_block *nfb, unsigned long action, void *hcpu)
>  {
> @@ -284,6 +305,9 @@ static int cpu_callback(
>      case CPU_STARTING:
>          psr_cpu_init(cpu);
>          break;
> +    case CPU_DYING:
> +        psr_cpu_fini(cpu);
> +        break;
>      }
>  
>      return NOTIFY_DONE;


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