Re: [PATCH v3 03/19] libxc: allocate memory with vNUMA information for PV guest

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 14/01/15 10:53, Wei Liu wrote:
>
>>> +            return -EINVAL;
>>> +        }
>>> +
>>>          /* allocate guest memory */
>>> -        for ( i = rc = allocsz = 0;
>>> -              (i < dom->total_pages) && !rc;
>>> -              i += allocsz )
>>> +        pfn_base = 0;
>>> +        for ( i = 0; i < dom->nr_vnodes; i++ )
>>>          {
>>> -            allocsz = dom->total_pages - i;
>>> -            if ( allocsz > 1024*1024 )
>>> -                allocsz = 1024*1024;
>>> -            rc = xc_domain_populate_physmap_exact(
>>> -                dom->xch, dom->guest_domid, allocsz,
>>> -                0, 0, &dom->p2m_host[i]);
>>> +            unsigned int memflags;
>>> +            uint64_t pages;
>>> +
>>> +            memflags = 0;
>>> +            if ( dom->vnode_to_pnode[i] != XC_VNUMA_NO_NODE )
>>> +            {
>>> +                memflags |= XENMEMF_exact_node(dom->vnode_to_pnode[i]);
>>> +                memflags |= XENMEMF_exact_node_request;
>>> +            }
>>> +
>>> +            pages = (dom->vnode_size[i] << 20) >> PAGE_SHIFT;
>>> +
>>> +            for ( j = 0; j < pages; j += allocsz )
>>> +            {
>>> +                allocsz = pages - j;
>>> +                if ( allocsz > 1024*1024 )
>>> +                    allocsz = 1024*1024;
>>> +
>>> +                rc = xc_domain_populate_physmap_exact(dom->xch,
>>> +                         dom->guest_domid, allocsz, 0, memflags,
>>> +                         &dom->p2m_host[pfn_base+j]);
>>> +
>>> +                if ( rc )
>>> +                {
>>> +                    if ( dom->vnode_to_pnode[i] != XC_VNUMA_NO_NODE )
>>> +                        xc_dom_panic(dom->xch, XC_INTERNAL_ERROR,
>>> +                                     "%s: fail to allocate 0x%"PRIx64"/0x%"PRIpfn" pages (v=%d, p=%d)\n",
>>> +                                     __FUNCTION__, pages, dom->total_pages, i,
>>> +                                     dom->vnode_to_pnode[i]);
>> "failed to allocate"
>>
> Fixed.
>
>> I am not sure the total number of pages is useful here, especially as
>> you don't know how many pages have successfully been allocated first.
>>
> Are you suggesting we don't print any number of print more numbers?

I am not suggesting that you don't print numbers.  "pages" is useful to
know in the error message, but "d->total_pages" is not, as you don't
know how many successful allocations have occurred before, and the total
allocated memory so far.

~Andrew