Re: [Xen-devel] [PATCH v3 6/7] xen: adds vNUMA info debug-key u

[Dario Faggioli <dario.faggioli@xxxxxxxxxx>]

On lun, 2013-11-18 at 15:24 -0500, Elena Ufimtseva wrote:
> Signed-off-by: Elena Ufimtseva <ufimtseva@xxxxxxxxx>

> diff --git a/xen/arch/x86/numa.c b/xen/arch/x86/numa.c
> index b141877..1cf74ee 100644
> @@ -389,6 +389,24 @@ static void dump_numa(unsigned char key)
>  
>               for_each_online_node(i)
>                       printk("    Node %u: %u\n", i, page_num_node[i]);
> +
> +             if (d->vnuma.nr_vnodes > 0)
> +             {
> +                     printk("    Domain has %d vnodes\n", 
> d->vnuma.nr_vnodes);
> +                     for (i = 0; i < d->vnuma.nr_vnodes; i++) {
> +                             printk("        vnode %d - pnode %d:", i,
                                          ^ I'd intend this a bit less,
but that's certainly a minor thing.

> +                                     d->vnuma.vnode_to_pnode[i] >= 
> MAX_NUMNODES ? -1 : d->vnuma.vnode_to_pnode[i]);
>
How long is this line above? :-P

> +                             printk(" %"PRIu64" MB \n",
> +                                        (d->vnuma.vmemrange[i].end - 
> d->vnuma.vmemrange[i].start) >> 20);
> +                     }
> +
> +                     printk("    Domain vcpu to vnode: \n");
> +                     for (i = 0; i < d->max_vcpus; i++)
> +                             printk("%s%d %s", i % 8 == 0 ? "    " : "",
> +                                     d->vnuma.vcpu_to_vnode[i],
> +                                     (i + 1) % 8 == 0 ? "\n" : "");
>
Do you have an example of the output for this? Looks like you're just
listing the vcpus, although in a specific order... How one is supposed
to tell what set of vcpus belong to what vnode?

Wouldn't it be clearer to print the list of vcpus belonging to each node
right after having printed its memory (i.e., in the for above)?

Also, regarding the '(i + 1) % 8 == 0 ? "\n" : ""' I found i rather
ugly. Itt may be a matter of personal taste, but I think something along
the line of:

  if ( !(i % 8) )
    printk("\n");

would be much more readable.

Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

Attachment: signature.asc
Description: This is a digitally signed message part

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