Re: [Xen-devel] [PATCH v2 01/19] xen: dump vNUMA information with debug key "u"

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 01.12.14 at 16:33, <wei.liu2@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/numa.c
> +++ b/xen/arch/x86/numa.c
> @@ -363,10 +363,13 @@ EXPORT_SYMBOL(node_data);
>  static void dump_numa(unsigned char key)
>  {
>      s_time_t now = NOW();
> -    int i;
> +    unsigned int i, j, n;
> +    int err;
>      struct domain *d;
>      struct page_info *page;
>      unsigned int page_num_node[MAX_NUMNODES];
> +    uint64_t mem;
> +    struct vnuma_info *vnuma;

If this can be const, it should be in a pure dumping function.

> @@ -408,6 +411,48 @@ 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 )
> +                continue;
> +
> +        vnuma = d->vnuma;
> +        printk("     %u vnodes, %u vcpus:\n", vnuma->nr_vnodes, 
> d->max_vcpus);
> +        for ( i = 0; i < vnuma->nr_vnodes; i++ )
> +        {
> +            err = snprintf(keyhandler_scratch, 12, "%3u",
> +                    vnuma->vnode_to_pnode[i]);
> +            if ( err < 0 || vnuma->vnode_to_pnode[i] == NUMA_NO_NODE )
> +                strlcpy(keyhandler_scratch, "???", 3);
> +
> +            printk("       vnode  %3u - pnode %s\n", i, keyhandler_scratch);
> +            for ( j = 0; j < vnuma->nr_vmemranges; j++ )
> +            {
> +                if ( vnuma->vmemrange[j].nid == i )
> +                {
> +                    mem = vnuma->vmemrange[j].end - 
> vnuma->vmemrange[j].start;
> +                    printk("%16"PRIu64" MB: %#016"PRIx64" - %#016"PRIx64"\n",

Am I misremembering that these were just "0x%"PRIx64 originally?
I ask because converting to the 0-padded fixed width form makes
no sense together with the # modifier. For these ranges I think it's
quite obvious that the numbers are hex, so I'd suggest dropping
the #s without replacement. And to be honest I'm also against
printing duplicate information: The memory range already specifies
how much memory this is.

> +                           mem >> 20,
> +                           vnuma->vmemrange[j].start,
> +                           vnuma->vmemrange[j].end);
> +                }
> +            }
> +
> +            printk("       vcpus: ");
> +            for ( j = 0, n = 0; j < d->max_vcpus; j++ )
> +            {
> +                if ( vnuma->vcpu_to_vnode[j] == i )
> +                {
> +                    if ( (n + 1) % 8 == 0 )
> +                        printk("%3d\n", j);
> +                    else if ( !(n % 8) && n != 0 )
> +                        printk("%17d ", j);
> +                    else
> +                        printk("%3d ", j);
> +                    n++;
> +                }

Please consider very-many-vCPU guests here - see Andrew's commit
9cf71226 ("process softirqs while dumping domains").

Jan


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