Re: [Xen-devel] [PATCH v7 03/14] x86: detect and initialize Intel CAT feature

[Chao Peng <chao.p.peng@xxxxxxxxxxxxxxx>]

On Tue, May 19, 2015 at 09:42:07AM +0100, Jan Beulich wrote:
> >>> On 19.05.15 at 09:40, <chao.p.peng@xxxxxxxxxxxxxxx> wrote:
> > On Mon, May 18, 2015 at 02:33:51PM +0100, Jan Beulich wrote:
> >> >>> On 08.05.15 at 10:56, <chao.p.peng@xxxxxxxxxxxxxxx> wrote:
> >> > +static unsigned long *__read_mostly cat_socket_init_bitmap;
> >> > +static unsigned long *__read_mostly cat_socket_enable_bitmap;
> >> 
> >> Didn't we agree to fold these two into one? Apart from that the
> >> _bitmap name tag doesn't seem very useful...
> > 
> > Looks like this?
> > 
> > static unsigned long *__read_mostly cat_socket_init,
> >                      *__read_mostly cat_socket_enable;
> 
> Yes, except that you will want to drop one (or make clear why you
> need both).

As said in the previous version, cat_socket_enable is the actual CAT
enabled status.

And cat_socket_init is used for performance optimization purpose only.
It indicate if the socket has been initialized, so that initialization
happens only once for each socket, but not __cpus_in_socket__ times.

There are three possibilities:
1)  Not initialized;
2)  Initialized, CAT disabled;
3)  Initialized, CAT enabled;

So it's not possible to use only one bit to represent three values.

> 
> >> >  static int cpu_callback(
> >> >      struct notifier_block *nfb, unsigned long action, void *hcpu)
> >> >  {
> >> >      if ( action == CPU_STARTING )
> >> >          psr_cpu_init();
> >> > +    else if ( action == CPU_DYING )
> >> > +        psr_cpu_fini();
> >> 
> >> Are these the right notifiers for doing things involving memory
> >> allocation / freeing?
> > 
> > psr_cpu_fini() does not really have memory allocation/freeing but
> > psr_cpu_init() does have.
> 
> Hmm, wait - where did I see this allocation? Looking again, I don't see
> it now. But if there was one, then surely it would be wrong for _fini
> to not free it.
> 
> > While one thing to clarify is: psr_cpu_init() will not propagate
> > the error even when the memory allocation fails(instead it disables
> > the CAT on the socket).
> > 
> > If there is still problem then perhaps I need change CPU_STARTING back
> > to CPU_ONLINE and make on_selected_cpus() call on target cpu.
> 
> No, if there was any allocation needed, you should do the allocation
> in CPU_UP_PREPARE and the initialization in CPU_STARTING. But it
> looks like this is moot now anyway.

As I will add allocation in patch 4, so I will move psr_cpu_init() to
CPU_UP_PREPARE(As the allocation length is based on the cpuid result, so
the initialization will also be done here), but still need to move
psr_cpu_fini(which will add the freeing code) to CPU_DOWN_PREPARE?

Chao

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