Hi Sharyathi
On Tue, Feb 02, 2010 at 10:46:41AM +0530, Sharyathi Nagesh wrote:
> Cliff
> Thank you for providing the correction. It looks like the best thing to
> do with changed buffer size context after adding earlier patch that I
> sent.
> I had one observation, though it doesn't impact this issue directly. In
> function copy_bitmask_to_bitmask() 3rd condition looked redundant to me.
> Since first 2 conditions cover all the cases, in that situation would
> these conditions make sense ?
> -----------------------------------------------------------------------------------------------
> else {
> bytes = CPU_BYTES(bmpfrom->size);
> memcpy(bmpto->maskp, bmpfrom->maskp, bytes);
> }
> -----------------------------------------------------------------------------------------------
Indeed extraneous. I removed them. Thanks.
> Do let us know when can we expect these patches upstream.
> Thank you
> Sharyathi
Your patch is included in numactl-2.0.4-rc2.tar.gz
(at ftp://oss.sgi.com/www/projects/libnuma/download/ )
-Cliff
>
> On 02/02/2010 04:07 AM, Cliff Wickman wrote:
>> Hi Sharyathi,
>>
>> Thanks for both patch and test case.
>>
>> The patch needs one more change I think.
>> The target buffer may be bigger, so the copy of the map needs
>> to be zero-extended.
>> Would you review it?
>>
>> Thx.
>> -Cliff
>>
>>> Date: Thu, 28 Jan 2010 11:23:05 +0530
>>> From: Sharyathi Nagesh<sharyath@xxxxxxxxxx>
>>> To: linux-numa@xxxxxxxxxxxxxxx, Andi Kleen<andi@xxxxxxxxxxxxxx>,
>>> Christoph Lameter<clameter@xxxxxxx>, Cliff Wickman<cpw@xxxxxxx>,
>>> Lee Schermerhorn<lee.schermerhorn@xxxxxx>,
>>> Amit K Arora<amitarora@xxxxxxxxxx>, deepti.kalra@xxxxxxxxxx
>>> Subject: Fix to numa_node_to_cpus_v2
>>>
>>> Hi
>>>
>>> We observed that numa_node_to_cpus api() api converts a node number to a
>>> bitmask of CPUs. The user must pass a long enough buffer. If the buffer is not
>>> long enough errno will be set to ERANGE and -1 returned. On success 0 is returned.
>>> This api has been changed in numa version 2.0. It has new implementation (_v2)
>>>
>>> Analysis:
>>> Now within the numa_node_to_cpus code there is a check if the size of buffer
>>> passed from the user matches the one returned by the sched_getaffinity. This
>>> check fails and hence we see "map size mismatch: abort" messages coming out on
>>> console. My system has 4 node and 8 CPUs.
>>>
>>> Testcase to reproduce the problem:
>>> #include<errno.h>
>>> #include<stdio.h>
>>> #include<stdlib.h>
>>> #include<numa.h>
>>>
>>> typedef unsigned long BUF[64];
>>>
>>> int numa_exit_on_error = 0;
>>>
>>> void node_to_cpus(void)
>>> {
>>> int i;
>>> BUF cpubuf;
>>> BUF affinityCPUs;
>>> int maxnode = numa_max_node();
>>> printf("available: %d nodes (0-%d)\n", 1+maxnode, maxnode);
>>> for (i = 0; i<= maxnode; i++) {
>>> printf("Calling numa_node_to_cpus()\n");
>>> printf("Size of BUF is : %d \n",sizeof(BUF));
>>> if ( 0 == numa_node_to_cpus(i, cpubuf, sizeof(BUF)) ) {
>>> printf("Calling numa_node_to_cpus() again \n");
>>> if ( 0 == numa_node_to_cpus(i, cpubuf, sizeof(BUF)) ) {
>>> } else {
>>> printf("Got< 0 \n");
>>> numa_error("numa_node_to_cpu");
>>> numa_exit_on_error = 1;
>>> exit(numa_exit_on_error);
>>> }
>>> } else {
>>> numa_error("numa_node_to_cpu 0");
>>> numa_exit_on_error = 1;
>>> exit(numa_exit_on_error);
>>> }
>>> }
>>> }
>>> int main()
>>> {
>>> void node_to_cpus();
>>> if (numa_available()< 0)
>>> {
>>> printf("This system does not support NUMA policy\n");
>>> numa_error("numa_available");
>>> numa_exit_on_error = 1;
>>> exit(numa_exit_on_error);
>>> }
>>> node_to_cpus();
>>> return numa_exit_on_error;
>>> }
>>> --------------------------------------------------------------------------------
>>>
>>> Problem Fix:
>>> The fix is to allow numa_node_to_cpus_v2() to fail only when the supplied
>>> buffer is smaller than the bitmask required to represent online NUMA nodes.
>>> Attaching the patch to address this issues, patch is generated against numactl-2.0.4-rc1
>>>
>>> Regards
>>> Yeehaw
>>>
>> ---
>> libnuma.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> Index: numactl-dev/libnuma.c
>> ===================================================================
>> --- numactl-dev.orig/libnuma.c
>> +++ numactl-dev/libnuma.c
>> @@ -1272,11 +1272,11 @@ numa_node_to_cpus_v2(int node, struct bi
>>
>> if (node_cpu_mask_v2[node]) {
>> /* have already constructed a mask for this node */
>> - if (buffer->size != node_cpu_mask_v2[node]->size) {
>> + if (buffer->size< node_cpu_mask_v2[node]->size) {
>> numa_error("map size mismatch; abort\n");
>> return -1;
>> }
>> - memcpy(buffer->maskp, node_cpu_mask_v2[node]->maskp, bufferlen);
>> + copy_bitmask_to_bitmask(node_cpu_mask_v2[node], buffer);
>> return 0;
>> }
>>
--
Cliff Wickman
SGI
cpw@xxxxxxx
(651) 683-3824
--
To unsubscribe from this list: send the line "unsubscribe linux-numa" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
