RE: [Xen-devel] [PATCH] Export Multicore information

["Kamble, Nitin A" <nitin.a.kamble@xxxxxxxxx>]

Hi Emmanuel,
  Good catch. I will incorporate your suggestion in the next patch.

Thanks & Regards,
Nitin 
Open Source Technology Center, Intel Corporation.
------------------------------------------------------------------------
-
The mind is like a parachute; it works much better when it's open.

>-----Original Message-----
>From: Emmanuel Ackaouy [mailto:ack@xxxxxxxxxxxxx]
>Sent: Wednesday, December 13, 2006 4:01 AM
>To: Kamble, Nitin A
>Cc: Keir Fraser; Ian Pratt; Mallick, Asit K; Yu, Wilfred; Xen devel
list;
>Yang, Fred; Nakajima, Jun
>Subject: Re: [Xen-devel] [PATCH] Export Multicore information
>
>On Tue, Dec 12, 2006 at 03:39:22PM -0800, Kamble, Nitin A wrote:
>> I think I should have added some documentation comments in the code
>> there mentioning the usage of count. It would make more sense if you
>> look at the code which is calling this interface in the xc.c. There
are
>> 2 conventions for calling this sysctl function.
>>
>> 1. With count = 0, and the array_list = NULL
>>
>> 2. Here user space allocates the space for array_list of count size,
and
>> passes it to the sysctl.
>
>(2) is the case I'm talking about.
>
>From your patch in sysctl.c:
>
>+        count = min((int)pi->count, num_possible_cpus());
>
>    ^^^^^^^^^^ You grab "count" correctly.
>
>+
>+        if ( guest_handle_is_null(pi->cpuinfo_list) ) {
>+            printk("sysctl XEN_SYSCTL_cpuinfo: guest handle is null
\n");
>+            ret = -EFAULT;
>+            break;
>+        }
>+
>+        j = 0;
>+        for_each_cpu(i) {
>+            extern int cpu_2_node[];
>+
>+            cl.cpu_id = i;
>+            cl.core_id = cpu_core_id[i];
>+            cl.package_id = phys_proc_id[i];
>+            cl.node_id = cpu_2_node[i];
>+            cl.thread_siblings_map = cpus_addr(cpu_sibling_map[i])[0];
>+            cl.core_siblings_map = cpus_addr(cpu_core_map[i])[0];
>+
>+            if ( copy_to_guest_offset(pi->cpuinfo_list, j, &cl, 1) ) {
>+                printk("sysctl XEN_SYSCTL_cpuinfo: copy to guest
>(cpuinfo_list)
> failed \n");
>+                ret -EFAULT;
>+                break;
>+            }
>+            j++;
>
>        ^^^^^^^^^^^ We never check that 'j' goes >= 'count'.
>
>+        }
>
>
>As a matter of fact, it looks to me like 'count' is a dead
>variable as soon as it's set?

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel