Add documentation to OS provider

include/umf/providers/provider_os_memory.h

@@ -16,47 +16,57 @@ extern "C" {
 
 #define UMF_OS_RESULTS_START_FROM 1000
 
+/// @brief Protection of the memory allocations
 typedef enum umf_mem_protection_flags_t {
-    UMF_PROTECTION_NONE = (1 << 0),
-    UMF_PROTECTION_READ = (1 << 1),
-    UMF_PROTECTION_WRITE = (1 << 2),
-    UMF_PROTECTION_EXEC = (1 << 3),
+    UMF_PROTECTION_NONE = (1 << 0),  ///< Memory allocations can not be accessed
+    UMF_PROTECTION_READ = (1 << 1),  ///< Memory allocations can be read.
+    UMF_PROTECTION_WRITE = (1 << 2), ///< Memory allocations can be written.
+    UMF_PROTECTION_EXEC = (1 << 3),  ///< Memory allocations can be executed.
 
     UMF_PROTECTION_MAX // must be the last one
 } umf_mem_protection_flags_t;
 
+/// @brief Visibility of the memory allocations
 typedef enum umf_mem_visibility_t {
-    UMF_VISIBILITY_SHARED,
-    UMF_VISIBILITY_PRIVATE,
+    UMF_VISIBILITY_SHARED, ///< Updates to the memory allocated using OS provider are visible to other processes.
+    /// TODO: need to expose functionality to share open the mapping in other process and explicit sync?
+    UMF_VISIBILITY_PRIVATE, ///<  Updates to the memory allocated using OS provider are not visible to other processes.
 } umf_mem_visibility_t;
 
+/// @brief Memory binding mode
+///
+/// Specifies how memory is bound to NUMA nodes on systems that support NUMA.
+/// Not every mode is supported on every system.
 typedef enum umf_numa_mode_t {
-    UMF_NUMA_MODE_DEFAULT,
-    UMF_NUMA_MODE_BIND,
-    UMF_NUMA_MODE_INTERLEAVE,
-    UMF_NUMA_MODE_PREFERRED,
-    UMF_NUMA_MODE_LOCAL,
+    UMF_NUMA_MODE_DEFAULT, ///< Default binding mode. Actual binding policy is system-specific.
+    ///  On linux this corresponds to MPOL_DEFAULT. If this mode is specified,
+    ///  nodemask must be NULL and maxnode must be 0.
+    UMF_NUMA_MODE_BIND, ///< Restricts memory allocation to nodes specified in nodemask. Allocations
+    ///  might come from any of the allowed nodes. Nodemask must specify at least one node.
+    UMF_NUMA_MODE_INTERLEAVE, ///< Interleaves memory allocations across the set of nodes specified in nodemask.
+                              /// Nodemask must specify at least one node.
+    UMF_NUMA_MODE_PREFERRED, ///< Specifies preferred node for allocation. If allocation cannot be fulfilled,
+    ///  memory will be allocated from other nodes.
+    UMF_NUMA_MODE_LOCAL, ///< The memory is allocated on the node of the CPU that triggered the allocation.
+    ///  If this mode is specified, nodemask must be NULL and maxnode must be 0.
+    /// TODO: should this be a hint or strict policy?
 } umf_numa_mode_t;
 
-typedef enum umf_purge_advise_t {
-    UMF_PURGE_LAZY,
-    UMF_PURGE_FORCE,
-} umf_purge_advise_t;
-
 /// @brief Memory provider settings struct
 typedef struct umf_os_memory_provider_params_t {
     /// combination of 'umf_mem_protection_flags_t' flags
     unsigned protection;
+
     /// shared or private visibility of memory mapped by a provider
-    /// sets MAP_SHARED and MAP_PRIVATE flags respectively on internal mmap() calls
     umf_mem_visibility_t visibility;
 
     // NUMA config
-    /// nodemask used in internal mbind() calls
+    /// points to a bit mask of nodes containing up to maxnode bits, depending on
+    /// selected numa_mode newly allocated memory will be bound to those nodes
     unsigned long *nodemask;
-    /// maximum number of nodes in \p nodemask
+    /// max number of bits in nodemask
     unsigned long maxnode;
-    /// flag that relates to one of the MPOL_* flags used in internal mbind() calls
+    /// describes how nodemask is interpreted
     umf_numa_mode_t numa_mode;
 
     // others

src/provider/provider_os_memory.c

@@ -146,8 +146,16 @@ static hwloc_membind_policy_t translate_numa_mode(umf_numa_mode_t mode,
         }
         return HWLOC_MEMBIND_DEFAULT;
     case UMF_NUMA_MODE_BIND:
+        if (nodemaskEmpty) {
+            // nodeset must not be empty
+            return -1;
+        }
         return HWLOC_MEMBIND_BIND;
     case UMF_NUMA_MODE_INTERLEAVE:
+        if (nodemaskEmpty) {
+            // nodeset must not be empty
+            return -1;
+        }
         return HWLOC_MEMBIND_INTERLEAVE;
     case UMF_NUMA_MODE_PREFERRED:
         return HWLOC_MEMBIND_BIND;

src/provider/provider_os_memory_internal.h

@@ -15,6 +15,11 @@
 extern "C" {
 #endif
 
+typedef enum umf_purge_advise_t {
+    UMF_PURGE_LAZY,
+    UMF_PURGE_FORCE,
+} umf_purge_advise_t;
+
 int os_translate_flags(unsigned in_flags, unsigned max,
                        int (*translate_flag)(unsigned));
 

test/provider_os_memory.cpp

@@ -126,21 +126,55 @@ static void test_alloc_failure(umf_memory_provider_handle_t provider,
 
 // negative tests for umfMemoryProviderCreate()
 
-TEST_F(test, create_WRONG_NUMA_MODE) {
+static umf_result_t create_os_provider_with_mode(umf_numa_mode_t mode,
+                                                 unsigned long *nodemask,
+                                                 unsigned long maxnode) {
     umf_result_t umf_result;
     umf_memory_provider_handle_t os_memory_provider = nullptr;
     umf_os_memory_provider_params_t os_memory_provider_params =
         umfOsMemoryProviderParamsDefault();
 
-    // NUMA binding mode not supported for UMF_VISIBILITY_SHARED
-    os_memory_provider_params.visibility = UMF_VISIBILITY_SHARED;
-    os_memory_provider_params.numa_mode = UMF_NUMA_MODE_BIND;
+    os_memory_provider_params.numa_mode = mode;
+    os_memory_provider_params.nodemask = nodemask;
+    os_memory_provider_params.maxnode = maxnode;
 
     umf_result = umfMemoryProviderCreate(umfOsMemoryProviderOps(),
                                          &os_memory_provider_params,
                                          &os_memory_provider);
-    ASSERT_EQ(umf_result, UMF_RESULT_ERROR_INVALID_ARGUMENT);
-    ASSERT_EQ(os_memory_provider, nullptr);
+    if (umf_result == UMF_RESULT_SUCCESS) {
+        EXPECT_NE(os_memory_provider, nullptr);
+        umfMemoryProviderDestroy(os_memory_provider);
+    } else {
+        EXPECT_EQ(os_memory_provider, nullptr);
+    }
+
+    return umf_result;
+}
+
+static unsigned long valid_nodemask = 0x1;
+static unsigned long valid_maxnode = 2;
+
+TEST_F(test, create_WRONG_NUMA_MODE_DEFAULT) {
+    auto ret = create_os_provider_with_mode(UMF_NUMA_MODE_DEFAULT,
+                                            &valid_nodemask, valid_maxnode);
+    ASSERT_EQ(ret, UMF_RESULT_ERROR_INVALID_ARGUMENT);
+}
+
+TEST_F(test, create_WRONG_NUMA_MODE_LOCAL) {
+    auto ret = create_os_provider_with_mode(UMF_NUMA_MODE_LOCAL,
+                                            &valid_nodemask, valid_maxnode);
+    ASSERT_EQ(ret, UMF_RESULT_ERROR_INVALID_ARGUMENT);
+}
+
+TEST_F(test, create_WRONG_NUMA_MODE_BIND) {
+    auto ret = create_os_provider_with_mode(UMF_NUMA_MODE_BIND, nullptr, 0);
+    ASSERT_EQ(ret, UMF_RESULT_ERROR_INVALID_ARGUMENT);
+}
+
+TEST_F(test, create_WRONG_NUMA_MODE_INTERLEAVE) {
+    auto ret =
+        create_os_provider_with_mode(UMF_NUMA_MODE_INTERLEAVE, nullptr, 0);
+    ASSERT_EQ(ret, UMF_RESULT_ERROR_INVALID_ARGUMENT);
 }
 
 // positive tests using test_alloc_free_success