Re: Fix to numa_node_to_cpus_v2

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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;
 	}
 
--
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

[Index of Archives]     [Linux Kernel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux SCSI]     [Devices]

  Powered by Linux