From mboxrd@z Thu Jan  1 00:00:00 1970
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Subject: Re: [PATCH RFC v2 7/7] xen/vNUMA: adds vNUMA to NUMA
 debug-key
Date: Fri, 27 Sep 2013 13:05:51 -0400
Message-ID: <20130927170551.GB11397@phenom.dumpdata.com>
References: <1379062242-13972-1-git-send-email-ufimtseva@gmail.com>
	<CAFLBxZbc3KxAadSxxHmWTC4uQM__pSMzbKPkCWSctJ4b6R8q+g@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Content-Disposition: inline
In-Reply-To: <CAFLBxZbc3KxAadSxxHmWTC4uQM__pSMzbKPkCWSctJ4b6R8q+g@mail.gmail.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>, Matt Wilson <msw@linux.com>, Dario Faggioli <dario.faggioli@citrix.com>, Li Yechen <lccycc123@gmail.com>, "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>, Elena Ufimtseva <ufimtseva@gmail.com>
List-Id: xen-devel@lists.xenproject.org

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