Re: [Xen-devel] [PATCH 2/6] x86: add support for COS/CBM manangement

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

On Mon, Mar 16, 2015 at 05:10:32PM +0000, Jan Beulich wrote:
> >>> On 13.03.15 at 11:13, <chao.p.peng@xxxxxxxxxxxxxxx> wrote:
> > +    else
> > +    {
> > +        unsigned int cpu = cpumask_check(get_socket_cpu(socket));
> 
> Isn't this going to trigger an assertion when the socket count got
> specified on the command line?

Yes, the assertion is needed in tools side anyway. Here the check seems
unnecessary.

> 
> > +    if( !d->arch.psr_cos_ids )
> > +        return;
> 
> Considering this check ...
> 
> > +    for ( socket = 0; socket < opt_socket_num; socket++)
> > +    {
> > +        cos = d->arch.psr_cos_ids[socket];
> > +        if ( cos == 0 )
> > +            continue;
> > +
> > +        map = cat_socket_info[socket].cos_cbm_map;
> > +        if ( map )
> > +            map[cos].ref--;
> > +    }
> > +
> > +    xfree(d->arch.psr_cos_ids);
> 
> ... I think you want to clear the pointer here.

This function is called by arch_domain_destroy() or the failure path of
arch_domain_create(). In both cases the domain structure will be freed
later so setting NULL is pointless.

> 
> > @@ -222,6 +422,17 @@ static void do_cat_cpu_init(void* data)
> >          info->cbm_len = (eax & 0x1f) + 1;
> 
> This means cbm_len <= 32. Why is cos_cbm_map[].cbm then a
> uint64_t?

Currently the cbm_len is EAX[4:0], 64 bits cbm here is for future
possible enhancement.

Chao

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