Re: [Xen-devel] [PATCH v3 12/19] hvmloader: retrieve vNUMA information from hypervisor

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Tue, Jan 13, 2015 at 04:50:11PM +0000, Jan Beulich wrote:
> >>> On 13.01.15 at 13:11, <wei.liu2@xxxxxxxxxx> wrote:
> > +void init_vnuma_info(void)
> > +{
> > +    int rc, retry = 0;
> > +    struct xen_vnuma_topology_info vnuma_topo;
> > +
> > +    vcpu_to_vnode = scratch_alloc(sizeof(uint32_t) * hvm_info->nr_vcpus, 
> > 0);
> 
> sizeof(*vcpu_to_vnode) please.
> 

Done.

> > +    rc = -EAGAIN;
> > +    while ( rc == -EAGAIN && retry < 10 )
> 
> What's the justification for 10 here? A sane tool stack shouldn't alter
> the values while starting the domain.
> 

I wasn't sure if a toolstack will change the values whilst domain is
running. But you now confirm that a sane toolstack shouldn't do that I
can just remove this loop.

> > +    {
> > +        vnuma_topo.domid = DOMID_SELF;
> > +        vnuma_topo.pad = 0;
> > +        vnuma_topo.nr_vcpus = 0;
> > +        vnuma_topo.nr_vnodes = 0;
> > +        vnuma_topo.nr_vmemranges = 0;
> > +
> > +        set_xen_guest_handle(vnuma_topo.vdistance.h, NULL);
> > +        set_xen_guest_handle(vnuma_topo.vcpu_to_vnode.h, NULL);
> > +        set_xen_guest_handle(vnuma_topo.vmemrange.h, NULL);
> > +
> > +        rc = hypercall_memory_op(XENMEM_get_vnumainfo, &vnuma_topo);
> > +
> > +        if ( rc == -EOPNOTSUPP )
> > +            return;
> > +
> > +        if ( rc != -ENOBUFS )
> > +            break;
> > +
> > +        ASSERT(vnuma_topo.nr_vcpus == hvm_info->nr_vcpus);
> 
> I also wonder whether we shouldn't make the hypervisor return
> back the (modified) values in the -EAGAIN error case, so that you
> could move above first half of the loop body out of the loop.
> 

I don't think hypercall modifies values in -EAGAIN case. The first half
of that loop is to prepare hypercall structure so that we can retrieve
the new size.

But since you say no sane toolstack should do that this issue becomes
moot.

Wei.

> Jan

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