Re: [Xen-devel] [PATCH v5 8/8] add vnuma info for debug-key

[Elena Ufimtseva <ufimtseva@xxxxxxxxx>]

On Tue, Jun 3, 2014 at 5:04 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:

>>> On 03.06.14 at 06:53, <ufimtseva@xxxxxxxxx> wrote:

This patch appears to be duplicated (with slightly different titles) in
the series - please fix this up for the next revision.

Jan, I will beÂresending theseÂpatches, thank you.

> --- 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, err;

"unsigned int" please for any variables used as array indexes or alike
and not explicitly needing to be signed for some reason.

> Â Â Â struct domain *d;
> Â Â Â struct page_info *page;
> + Â Â char tmp[12];

Are you perhaps unaware of keyhandler_scratch[]?

> Â Â Â unsigned int page_num_node[MAX_NUMNODES];
>
> Â Â Â printk("'%c' pressed -> dumping numa info (now-0x%X:%08X)\n", key,
> @@ -389,6 +390,32 @@ static void dump_numa(unsigned char key)
>
> Â Â Â Â Â Â Â for_each_online_node(i)
> Â Â Â Â Â Â Â Â Â Â Â printk(" Â ÂNode %u: %u\n", i, page_num_node[i]);
> +
> + Â Â Â Â Â Â printk(" Â ÂDomain has %u vnodes\n", d->vnuma.nr_vnodes);
> + Â Â Â Â Â Â for ( i = 0; i < d->vnuma.nr_vnodes; i++ ) {
> + Â Â Â Â Â Â Â Â Â Â err = snprintf(tmp, 12, "%u", d->vnuma.vnode_to_pnode[i]);
> + Â Â Â Â Â Â Â Â Â Â if ( err < 0 )
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â printk(" Â Â Â Âvnode %u - pnode %s,", i, "any");
> + Â Â Â Â Â Â Â Â Â Â else
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â printk(" Â Â Â Âvnode %u - pnode %s,", i,
> + Â Â Â Â Â Â Â Â Â Â d->vnuma.vnode_to_pnode[i] == NUMA_NO_NODE ? "any" : tmp);

Broken indentation.

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

Here too.

Â

The formatting in numa.c is utterly confusing. It seems I cant never get it right.

I have tried to follow the file indentation, but it should be more to it if I still cant get it right.

I have verified the formatting with vi command :set list and it looked good to me.

Also I used ÂhereÂ:setÂnoet ci pi sts=0 sw=4 ts=4.

> + Â Â Â Â Â Â Â Â Â Â printk("vcpus: ");
> +
> + Â Â Â Â Â Â Â Â Â Â for ( j = 0, n = 0; j < d->max_vcpus; j++ ) {
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â if ( d->vnuma.vcpu_to_vnode[j] == i ) {
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â if ( !((n + 1) % 8) )
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â printk("%u\n", j);
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â else if ( !(n % 8) && n != 0 )
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â printk("%s%u ", " Â Â Â Â Â Â ", j);
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â else
> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â printk("%u ", j);

And yet more of them. Also the sequence of leading spaces would better
be expressed with %8u or some such, but from the looks of it properly
aligning the output here would require all of this to be done a little
differently anyway.

Ok, let me see what I can do here.Â

Jan

--
Elena

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