Re: [PATCH RFC v2 7/7] xen/vNUMA: adds vNUMA to NUMA debug-key

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Thu, Sep 19, 2013 at 03:38:29PM +0100, George Dunlap wrote:
> On Fri, Sep 13, 2013 at 9:50 AM, Elena Ufimtseva <ufimtseva@gmail.com> wrote:
> > Prints basic information about vNUMA topology
> > for vNUMA enabled domains when issuing debug-key 'u'.
> >
> > Signed-off-by: Elena Ufimtseva <ufimtseva@gmail.com>
> > ---
> >  xen/arch/x86/numa.c |   23 ++++++++++++++++++++++-
> >  1 file changed, 22 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen/arch/x86/numa.c b/xen/arch/x86/numa.c
> > index b141877..7980e54 100644
> > --- a/xen/arch/x86/numa.c
> > +++ b/xen/arch/x86/numa.c
> > @@ -347,7 +347,7 @@ EXPORT_SYMBOL(node_data);
> >  static void dump_numa(unsigned char key)
> >  {
> >         s_time_t now = NOW();
> > -       int i;
> > +       int i, j;
> >         struct domain *d;
> >         struct page_info *page;
> >         unsigned int page_num_node[MAX_NUMNODES];
> > @@ -389,6 +389,27 @@ static void dump_numa(unsigned char key)
> >
> >                 for_each_online_node(i)
> >                         printk("    Node %u: %u\n", i, page_num_node[i]);
> 
> Blank line
> 
> > +                if(d->vnuma.nr_vnodes > 0)

I think you need a space there between the 'if' and '('..
> > +                {
> > +                    printk("    Domain has %d vnodes\n", d->vnuma.nr_vnodes);
> 
> Blank line, &c
> 
> > +                    for_each_online_node(i)
> > +                    {
> > +
> 
> Remove this blank line. :-)
> 
> > +                        printk("        pnode %d: vnodes: ", i);
> > +                        for(j = 0; j < d->vnuma.nr_vnodes; j++) {
> > +                            if (d->vnuma.vnode_to_pnode[j] == i)
> > +                            printk("%d (%Lu), ", j, (unsigned long long)
> > +                                                   (d->vnuma.vnuma_memblks[j].end -
> > +                                                   d->vnuma.vnuma_memblks[j].start)
> > +                                                   >> 20);
> 
> I think here it probable makes sense to be more vnuma-centric rather
> than pnuma centric: Rather than print the vnodes sorted by pnodes,
> just print the vnodes and print the pnode associated with it.  That
> gets rid of the need for the extra nest of the loop.
> 
> Other than that (and the blank lines) it looks good.
> 
>  -George
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel