Merge pull request #595 from bgoglin/l0-always-sysman

levelzero: only use Sysman queries instead of similar Core API queries

Author: bgoglin

hwloc/topology-levelzero.c

@@ -63,18 +63,21 @@ hwloc__levelzero_osdev_array_find(struct hwloc_osdev_array *array,
 
 static void
 hwloc__levelzero_properties_get(ze_device_handle_t zeh, zes_device_handle_t zesh,
-                                hwloc_obj_t osdev,
-                                int *is_integrated_p)
+                                hwloc_obj_t osdev, ze_device_properties_t *prop)
 {
   ze_result_t res;
-  ze_device_properties_t prop;
+  ze_device_properties_t _prop;
   zes_device_properties_t prop2;
   int is_subdevice = 0;
-  int is_integrated = 0;
 
-  memset(&prop, 0, sizeof(prop));
-  res = zeDeviceGetProperties(zeh, &prop);
-  if (res == ZE_RESULT_SUCCESS) {
+  if (!prop) {
+    /* no properties were given, get ours */
+    memset(&_prop, 0, sizeof(_prop));
+    res = zeDeviceGetProperties(zeh, &_prop);
+    if (res == ZE_RESULT_SUCCESS)
+      prop = &_prop;
+  }
+  if (prop) {
     /* name is the model name followed by the deviceID
      * flags 1<<0 means integrated (vs discrete).
      */
@@ -83,41 +86,35 @@ hwloc__levelzero_properties_get(ze_device_handle_t zeh, zes_device_handle_t zesh
     unsigned i;
     const char *type;
 
-    switch (prop.type) {
+    switch (prop->type) {
     case ZE_DEVICE_TYPE_GPU: type = "GPU"; break;
     case ZE_DEVICE_TYPE_CPU: type = "CPU"; break;
     case ZE_DEVICE_TYPE_FPGA: type = "FPGA"; break;
     case ZE_DEVICE_TYPE_MCA: type = "MCA"; break;
     case ZE_DEVICE_TYPE_VPU: type = "VPU"; break;
     default:
       if (HWLOC_SHOW_ALL_ERRORS())
-        fprintf(stderr, "hwloc/levelzero: unexpected device type %u\n", (unsigned) prop.type);
+        fprintf(stderr, "hwloc/levelzero: unexpected device type %u\n", (unsigned) prop->type);
       type = "Unknown";
     }
     hwloc_obj_add_info(osdev, "LevelZeroDeviceType", type);
-    snprintf(tmp, sizeof(tmp), "%u", prop.numSlices);
+    snprintf(tmp, sizeof(tmp), "%u", prop->numSlices);
     hwloc_obj_add_info(osdev, "LevelZeroNumSlices", tmp);
-    snprintf(tmp, sizeof(tmp), "%u", prop.numSubslicesPerSlice);
+    snprintf(tmp, sizeof(tmp), "%u", prop->numSubslicesPerSlice);
     hwloc_obj_add_info(osdev, "LevelZeroNumSubslicesPerSlice", tmp);
-    snprintf(tmp, sizeof(tmp), "%u", prop.numEUsPerSubslice);
+    snprintf(tmp, sizeof(tmp), "%u", prop->numEUsPerSubslice);
     hwloc_obj_add_info(osdev, "LevelZeroNumEUsPerSubslice", tmp);
-    snprintf(tmp, sizeof(tmp), "%u", prop.numThreadsPerEU);
+    snprintf(tmp, sizeof(tmp), "%u", prop->numThreadsPerEU);
     hwloc_obj_add_info(osdev, "LevelZeroNumThreadsPerEU", tmp);
 
     for(i=0; i<ZE_MAX_DEVICE_UUID_SIZE; i++)
-      snprintf(uuid+2*i, 3, "%02x", prop.uuid.id[i]);
+      snprintf(uuid+2*i, 3, "%02x", prop->uuid.id[i]);
     hwloc_obj_add_info(osdev, "LevelZeroUUID", uuid);
 
-    if (prop.flags & ZE_DEVICE_PROPERTY_FLAG_SUBDEVICE)
+    if (prop->flags & ZE_DEVICE_PROPERTY_FLAG_SUBDEVICE)
       is_subdevice = 1;
-
-    if (prop.flags & ZE_DEVICE_PROPERTY_FLAG_INTEGRATED)
-      is_integrated = 1;
   }
 
-  if (is_integrated_p)
-    *is_integrated_p = is_integrated;
-
   if (is_subdevice)
     /* sysman API on subdevice returns the same as root device, and we don't need those duplicate attributes */
     return;
@@ -172,9 +169,9 @@ hwloc__levelzero_cqprops_get(ze_device_handle_t zeh,
 }
 
 static int
-hwloc__levelzero_memory_get_from_sysman(zes_device_handle_t zesh,
-                                        hwloc_obj_t root_osdev,
-                                        unsigned nr_osdevs, hwloc_obj_t *sub_osdevs)
+hwloc__levelzero_memory_get(zes_device_handle_t zesh,
+                            hwloc_obj_t root_osdev,
+                            unsigned nr_osdevs, hwloc_obj_t *sub_osdevs)
 {
   zes_mem_handle_t *mh;
   uint32_t nr_mems;
@@ -279,95 +276,6 @@ hwloc__levelzero_memory_get_from_sysman(zes_device_handle_t zesh,
   return 0;
 }
 
-static void
-hwloc__levelzero_memory_get_from_coreapi(ze_device_handle_t zeh,
-                                         hwloc_obj_t osdev,
-                                         int ignore_ddr)
-{
-  ze_device_memory_properties_t *mh;
-  uint32_t nr_mems;
-  ze_result_t res;
-
-  nr_mems = 0;
-  res = zeDeviceGetMemoryProperties(zeh, &nr_mems, NULL);
-  if (res != ZE_RESULT_SUCCESS || !nr_mems)
-    return;
-  hwloc_debug("L0/CoreAPI: found %u memories in osdev %s\n",
-              nr_mems, osdev->name);
-
-  mh = malloc(nr_mems * sizeof(*mh));
-  if (mh) {
-    res = zeDeviceGetMemoryProperties(zeh, &nr_mems, mh);
-    if (res == ZE_RESULT_SUCCESS) {
-      unsigned m;
-      for(m=0; m<nr_mems; m++) {
-        const char *_name = mh[m].name;
-        char name[300], value[64];
-        /* FIXME: discrete GPUs report 95% of the physical memory (what sysman sees)
-         * while integrated GPUs report 80% of the host RAM (sysman sees 0), adjust?
-         */
-        hwloc_debug("L0/CoreAPI: found memory name %s size %llu in osdev %s\n",
-                    mh[m].name, (unsigned long long) mh[m].totalSize, osdev->name);
-        if (!mh[m].totalSize)
-          continue;
-        if (ignore_ddr && !strcmp(_name, "DDR"))
-          continue;
-        if (!_name[0])
-          _name = "Memory";
-        snprintf(name, sizeof(name), "LevelZero%sSize", _name); /* HBM or DDR, or Memory if unknown */
-        snprintf(value, sizeof(value), "%lluKiB", (unsigned long long) mh[m].totalSize >> 10);
-        hwloc_obj_add_info(osdev, name, value);
-      }
-    }
-    free(mh);
-  }
-}
-
-
-static void
-hwloc__levelzero_memory_get(ze_device_handle_t zeh, zes_device_handle_t zesh,
-                            hwloc_obj_t root_osdev, int is_integrated,
-                            unsigned nr_subdevices, zes_device_handle_t *subzehs, hwloc_obj_t *sub_osdevs)
-{
-  static int memory_from_coreapi = -1; /* 1 means coreapi, 0 means sysman, -1 means sysman if available or coreapi otherwise */
-  static int first = 1;
-
-  if (first) {
-    char *env;
-    env = getenv("HWLOC_L0_COREAPI_MEMORY");
-    if (env)
-      memory_from_coreapi = atoi(env);
-
-    if (memory_from_coreapi == -1) {
-      int ret = hwloc__levelzero_memory_get_from_sysman(zesh, root_osdev, nr_subdevices, sub_osdevs);
-      if (!ret) {
-        /* sysman worked, we're done, disable coreapi for next time */
-        hwloc_debug("levelzero: sysman/memory succeeded, disabling coreapi memory queries\n");
-        memory_from_coreapi = 0;
-        return;
-      }
-      /* sysman failed, enable coreapi */
-      hwloc_debug("levelzero: sysman/memory failed, enabling coreapi memory queries\n");
-      memory_from_coreapi = 1;
-    }
-
-    first = 0;
-  }
-
-  if (memory_from_coreapi > 0) {
-    unsigned k;
-    int ignore_ddr = (memory_from_coreapi != 2) && is_integrated; /* DDR ignored in integrated GPUs, it's like the host DRAM */
-    hwloc__levelzero_memory_get_from_coreapi(zeh, root_osdev, ignore_ddr);
-    for(k=0; k<nr_subdevices; k++)
-      hwloc__levelzero_memory_get_from_coreapi(subzehs[k], sub_osdevs[k], ignore_ddr);
-  } else {
-    hwloc__levelzero_memory_get_from_sysman(zesh, root_osdev, nr_subdevices, sub_osdevs);
-    /* no need to call hwloc__levelzero_memory_get() on subdevices,
-     * the call on the root device is enough (and identical to a call on subdevices)
-     */
-  }
-}
-
 struct hwloc_levelzero_ports {
   unsigned nr_allocated;
   unsigned nr;
@@ -551,7 +459,6 @@ hwloc__levelzero_devices_get(struct hwloc_topology *topology,
       hwloc_obj_t osdev, parent, *subosdevs = NULL;
       ze_device_properties_t props;
       zes_uuid_t uuid;
-      int is_integrated = 0;
       ze_bool_t onSubdevice = 0;
       uint32_t subdeviceId = 0;
 
@@ -583,7 +490,7 @@ hwloc__levelzero_devices_get(struct hwloc_topology *topology,
       snprintf(buffer, sizeof(buffer), "%u", j);
       hwloc_obj_add_info(osdev, "LevelZeroDriverDeviceIndex", buffer);
 
-      hwloc__levelzero_properties_get(zeh, zesh, osdev, &is_integrated);
+      hwloc__levelzero_properties_get(zeh, zesh, osdev, &props);
 
       hwloc__levelzero_cqprops_get(zeh, osdev);
 
@@ -641,7 +548,7 @@ hwloc__levelzero_devices_get(struct hwloc_topology *topology,
       }
 
       /* get all memory info at once */
-      hwloc__levelzero_memory_get(zeh, zesh, osdev, is_integrated, nr_subdevices, subzehs, subosdevs);
+      hwloc__levelzero_memory_get(zesh, osdev, nr_subdevices, subosdevs);
 
       /* get all ports info at once */
       if (!(hwloc_topology_get_flags(topology) & HWLOC_TOPOLOGY_FLAG_NO_DISTANCES))

include/hwloc/levelzero.h

@@ -166,6 +166,10 @@ hwloc_levelzero_get_sysman_device_cpuset(hwloc_topology_t topology __hwloc_attri
  * topology. If not, the locality of the object may still be found using
  * hwloc_levelzero_get_device_cpuset().
  *
+ * \note If the input ZE device is actually a subdevice, then its parent
+ * (root device) is actually translated, i.e. the main hwloc OS device
+ * is returned instead of one of its children.
+ *
  * \note The corresponding hwloc PCI device may be found by looking
  * at the result parent pointer (unless PCI devices are filtered out).
  *
@@ -231,6 +235,10 @@ hwloc_levelzero_get_device_osdev(hwloc_topology_t topology, ze_device_handle_t d
  * topology. If not, the locality of the object may still be found using
  * hwloc_levelzero_get_device_cpuset().
  *
+ * \note If the input ZES device is actually a subdevice, then its parent
+ * (root device) is actually translated, i.e. the main hwloc OS device
+ * is returned instead of one of its children.
+ *
  * \note The corresponding hwloc PCI device may be found by looking
  * at the result parent pointer (unless PCI devices are filtered out).
  */

tests/hwloc/levelzero.c

@@ -164,7 +164,7 @@ int main(void)
     sdvh = malloc(nbdevices * sizeof(*sdvh));
     if (!sdvh)
       continue;
-    res = zeDeviceGet(sdrh[i], &nbdevices, sdvh);
+    res = zesDeviceGet(sdrh[i], &nbdevices, sdvh);
     if (res != ZE_RESULT_SUCCESS) {
       free(sdvh);
       continue;
@@ -185,18 +185,17 @@ int main(void)
       printf("found OSDev %s\n", osdev->name);
       err = strncmp(osdev->name, "ze", 2);
       assert(!err);
-      assert(atoi(osdev->name+2) == (int) k);
+      /* don't check the index,
+       * ZE and ZES device orders may be different inside a single driver.
+       */
 
       assert(osdev->attr->osdev.types == (HWLOC_OBJ_OSDEV_COPROC|HWLOC_OBJ_OSDEV_GPU));
 
       assert(has_levelzero_backend);
 
-      value = hwloc_obj_get_info_by_name(osdev, "LevelZeroDriverIndex");
-      assert(value);
-      assert(atoi(value) == (int) i);
-      value = hwloc_obj_get_info_by_name(osdev, "LevelZeroDriverDeviceIndex");
-      assert(value);
-      assert(atoi(value) == (int) j);
+      /* don't check LevelZeroDriverIndex and LevelZeroDriverDeviceIndex,
+       * ZE and ZES device orders may be different inside a single driver.
+       */
 
       set = hwloc_bitmap_alloc();
       err = hwloc_levelzero_get_sysman_device_cpuset(topology, sdvh[j], set);

tests/hwloc/ports/include/levelzero/level_zero/ze_api.h

@@ -28,7 +28,6 @@ typedef enum _ze_device_type {
   ZE_DEVICE_TYPE_VPU = 5
 } ze_device_type_t;
 
-#define ZE_DEVICE_PROPERTY_FLAG_INTEGRATED (1<<0)
 #define ZE_DEVICE_PROPERTY_FLAG_SUBDEVICE (1<<1)
 
 #define ZE_MAX_DEVICE_UUID_SIZE 16
@@ -58,13 +57,6 @@ extern ze_result_t zeDeviceGetCommandQueueGroupProperties(ze_driver_handle_t, ui
 
 extern ze_result_t zeDeviceGetSubDevices(ze_device_handle_t, uint32_t *, ze_device_handle_t*);
 
-typedef struct ze_device_memory_properties {
-  uint64_t totalSize;
-  char *name;
-} ze_device_memory_properties_t;
-
-extern ze_result_t zeDeviceGetMemoryProperties(ze_device_handle_t, uint32_t *, ze_device_memory_properties_t*);
-
 typedef struct ze_pci_address_ext {
   uint32_t domain, bus, device, function;
 } ze_pci_address_ext_t;