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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 04.12.13 at 06:47, Elena Ufimtseva <ufimtseva@xxxxxxxxx> wrote:
> --- a/xen/arch/x86/numa.c
> +++ b/xen/arch/x86/numa.c
> @@ -347,9 +347,10 @@ EXPORT_SYMBOL(node_data);
>  static void dump_numa(unsigned char key)
>  {
>       s_time_t now = NOW();
> -     int i;
> +     int i, j, n;
>       struct domain *d;
>       struct page_info *page;
> +        char tmp[6];

Indentation.

> @@ -389,6 +390,33 @@ 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)
> +             {

Coding style (using Linux style in this file).

> +                     printk("    Domain has %d vnodes\n", 
> d->vnuma.nr_vnodes);
> +                     for (i = 0; i < d->vnuma.nr_vnodes; i++) {
> +                             n = 0;
> +                             snprintf(tmp, 5, "%d", 
> d->vnuma.vnode_to_pnode[i]);

vnode_to_pnode[] is an array of "unsigned int", so how come the
formatted result is (a) signed and (b) fitting in 5 characters? Apart
from that what you pass here is the array size, not one less than
it.

> +                             printk("        vnode %d - pnode %s", i,
> +                                             d->vnuma.vnode_to_pnode[i] >= 
> MAX_NUMNODES ? "any" : tmp);

Indentation again.

> +                             printk(" %"PRIu64" MB, ",
> +                                             (d->vnuma.vmemrange[i].end - 
> d->vnuma.vmemrange[i].start) >> 0);

Here too.

> +                             printk("vcpus: ");
> +                             for (j = 0; j < d->max_vcpus; j++) {

Setting n to zero would better be put in this loop header.

> +                                     if (d->vnuma.vcpu_to_vnode[j] == i) {
> +                                             if (!((n + 1) % 8))
> +                                                     printk("%d\n", j);
> +                                             else {
> +                                                     if ( !(n % 8) && n != 0 
> )

Please combine these into an "else if", lowering the level of indentation.

> +                                                             printk("%s%d ", 
> "             ", j);
> +                                                     else
> +                                                             printk("%d ", 
> j);
> +                                             }
> +                                             n++;
> +                                     }
> +                             }
> +                             printk("\n");
> +                     }
> +             }
>       }
>  
>       rcu_read_unlock(&domlist_read_lock);
> -- 
> 1.7.10.4




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