Re: [Xen-devel] [PATCH 2/2] libxc: fix PV vNUMA guest memory allocation

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Mon, Jul 06, 2015 at 02:33:35PM +0100, Ross Lagerwall wrote:
> On 07/06/2015 02:30 PM, Wei Liu wrote:
> >On Mon, Jul 06, 2015 at 02:26:37PM +0100, Andrew Cooper wrote:
> >>On 06/07/15 14:17, Wei Liu wrote:
> >>>In 415b58c1 (tools/libxc: Batch memory allocations for PV guests) the
> >>>number of super pages is calculated with the number of total pages. That
> >>>is wrong. It breaks PV guest vNUMA. The correct number of super pages
> >>>should be derived from the number of pages within that virtual NUMA
> >>>node.
> >>>
> >>>Also change the name and type of super page variable to match the naming
> >>>convention and type of normal page variable. Make the necessary
> >>>adjustment to make code compile.
> >>>
> >>>Reported-by: Dario Faggioli <dario.faggioli@xxxxxxxxxx>
> >>>Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
> >>>---
> >>>Cc: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
> >>>---
> >>>  tools/libxc/xc_dom_x86.c | 10 +++++-----
> >>>  1 file changed, 5 insertions(+), 5 deletions(-)
> >>>
> >>>diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
> >>>index acd7b3f..faabe96 100644
> >>>--- a/tools/libxc/xc_dom_x86.c
> >>>+++ b/tools/libxc/xc_dom_x86.c
> >>>@@ -879,9 +879,8 @@ int arch_setup_meminit(struct xc_dom_image *dom)
> >>>          for ( i = 0; i < nr_vmemranges; i++ )
> >>>          {
> >>>              unsigned int memflags;
> >>>-            uint64_t pages;
> >>>+            uint64_t pages, super_pages;
> >>>              unsigned int pnode = vnode_to_pnode[vmemranges[i].nid];
> >>>-            int nr_spages = dom->total_pages >> SUPERPAGE_PFN_SHIFT;
> >>>              xen_pfn_t extents[SUPERPAGE_BATCH_SIZE];
> >>>              xen_pfn_t pfn_base_idx;
> >>>
> >>>@@ -891,15 +890,16 @@ int arch_setup_meminit(struct xc_dom_image *dom)
> >>>
> >>>              pages = (vmemranges[i].end - vmemranges[i].start)
> >>>                  >> PAGE_SHIFT;
> >>>+            super_pages = pages >> SUPERPAGE_PFN_SHIFT;
> >>>              pfn_base = vmemranges[i].start >> PAGE_SHIFT;
> >>>
> >>>              for ( pfn = pfn_base; pfn < pfn_base+pages; pfn++ )
> >>>                  dom->p2m_host[pfn] = pfn;
> >>>
> >>>              pfn_base_idx = pfn_base;
> >>>-            while (nr_spages) {
> >>>-                int count = min(nr_spages, SUPERPAGE_BATCH_SIZE);
> >>>-                nr_spages -= count;
> >>>+            while (super_pages) {
> >>>+                int count = min_t(uint64_t, super_pages, 
> >>>SUPERPAGE_BATCH_SIZE);
> >>
> >>This line alone indicates that int is not an appropriate type for
> >>count.  unsigned int as as an absolute minimum, as it will be bounded to
> >>SUPERPAGE_BATCH_SIZE.
> >>
> >
> >Yes, I also need to make count uint64_t.
> >
> 
> Is this needed? count should never be larger than 512 (aka
> SUPERPAGE_BATCH_SIZE).
> 

Maybe uint64_t is overkill but I want to be as consistent as possible.

Wei.

> -- 
> Ross Lagerwall

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