Re: [Xen-devel] [PATCH v10 02/13] x86: detect and initialize Intel CAT feature

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

On Tue, Jul 07, 2015 at 11:25:46AM +0100, Jan Beulich wrote:
> >>> On 26.06.15 at 10:43, <chao.p.peng@xxxxxxxxxxxxxxx> wrote:
> > Detect Intel Cache Allocation Technology(CAT) feature and store the
> > cpuid information for later use. Currently only L3 cache allocation is
> > supported. The L3 CAT features may vary among sockets so per-socket
> > feature information is stored. The initialization can happen either at
> > boot time or when CPU(s) is hot plugged after booting.
> > 
> > Signed-off-by: Chao Peng <chao.p.peng@xxxxxxxxxxxxxxx>
> > Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > ---
> > Changes in v0:
> > * Add comment for cpu notification priority.
> 
> I'm afraid you did this just mechanically, while I implied you to also
> verify that what you chose is correct:
> 
> >  static struct notifier_block cpu_nfb = {
> > -    .notifier_call = cpu_callback
> > +    .notifier_call = cpu_callback,
> > +    /*
> > +     * Ensure socket_cpumask is still valid in CPU_DEAD notification
> > +     * (E.g. the CPU_DEAD notification should be called ahead of
> > +     * cpu_smpboot_free).
> > +     */
> > +    .priority = 1
> 
> CPU_DEAD is a NOTIFY_REVERSE notification, i.e. lowest priority
> first. I.e. your comment now contradicts the code afaict. Unless
> I'm mistaken with that, I assume you didn't really test this code
> path? Since everything else is fine with this patch, I'd be fine
> with doing the adjustment while committing, but I need your
> confirmation which variant actually works.
> 

Sorry for this. But It's really your change is correct.

Also see your typo fix, looks much better and thanks.

Chao

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