Open MPI logo

Hardware Locality Development Mailing List Archives

  |   Home   |   Support   |   FAQ   |   all Hardware Locality Development mailing list

Subject: Re: [hwloc-devel] The de-C99 of hwloc
From: Samuel Thibault (samuel.thibault_at_[hidden])
Date: 2011-02-17 18:53:07


Jeff Squyres, le Thu 17 Feb 2011 23:15:26 +0100, a écrit :
> - uint64_t cacheconfig[n];
> - uint32_t cacheconfig32[n];
> - uint64_t cachesize[n];
> + uint64_t *cacheconfig = NULL;
> + uint64_t *cachesize = NULL;
> + uint32_t *cacheconfig32 = NULL;
>
> + cacheconfig = malloc(sizeof(uint64_t) * n * 2);
> + if (NULL == cacheconfig) {
> + goto out;
> + }
> + cachesize = cacheconfig + n;

I'm not really fond of allocating for two pointers like this.

> hwloc_linux_get_pid_cpubind(hwloc_topology_t topology, pid_t pid, hwloc_bitmap_t hwloc_set, int flags)
> {
> hwloc_bitmap_t tidset = hwloc_bitmap_alloc();
> - hwloc_bitmap_t cpusets[2] = { hwloc_set, tidset };

Ow, even that is not allowed before C99?!

> + /* Setup ordering arrays */
> + if (!orders_initialized) {
> + obj_type_order[HWLOC_OBJ_SYSTEM] = 0;
> + obj_type_order[HWLOC_OBJ_MACHINE] = 1;
> + obj_type_order[HWLOC_OBJ_GROUP] = 2;
> + obj_type_order[HWLOC_OBJ_NODE] = 3;
> + obj_type_order[HWLOC_OBJ_SOCKET] = 4;
> + obj_type_order[HWLOC_OBJ_CACHE] = 5;
> + obj_type_order[HWLOC_OBJ_CORE] = 6;
> + obj_type_order[HWLOC_OBJ_PU] = 7;
> + obj_type_order[HWLOC_OBJ_MISC] = 8;
> +
> + orders_initialized = 1;
> + }

This is not actually thread safe. This needs a CPU write memory barrier
between the last obj_type_order[] array write and the orders_initialized
write, and a read barrier after orders_initialized is read as being
1, to make sure that a cpu seeing orders_initialized as 1 will always
also see the values in obj_type_order. A simpler way is to use
pthread_once().

I'll have to have a closer look on the changes, just reading the diff is
not really safe enough without more context around :)

Samuel