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

[Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>]

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).
--
Ross Lagerwall

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