public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH v2 0/6] libgomp: OpenMP pinned memory omp_alloc
@ 2023-08-23 14:14 Andrew Stubbs
  2023-08-23 14:14 ` [PATCH v2 1/6] libgomp: basic pinned memory on Linux Andrew Stubbs
                   ` (5 more replies)
  0 siblings, 6 replies; 15+ messages in thread
From: Andrew Stubbs @ 2023-08-23 14:14 UTC (permalink / raw)
  To: gcc-patches

This patch series is a rework of part of the series I posted about a year
ago:

https://patchwork.sourceware.org/project/gcc/list/?series=10748&state=%2A&archive=both

The series depends on the low-latency patch series I posted a few weeks
ago:

https://patchwork.sourceware.org/project/gcc/list/?series=23045&state=%2A&archive=both

I will post the Unified Shared Memory and allocator directive patches at
a later time.

This version of the patches implement the same basic features, rebased
on the current sourcebase, plus a Cuda-specific allocator for improved
performance with NVPTX offloading, and a custom allocator for better
handling of small allocations. The whole series has been bug fixed and
generally improved (mostly by Thomas :) ).

An older, less compact, version of these patches is already applied to
the devel/omp/gcc-13 (OG13) branch.

OK for mainline?

Andrew

Andrew Stubbs (5):
  libgomp: basic pinned memory on Linux
  libgomp, openmp: Add ompx_pinned_mem_alloc
  openmp: Add -foffload-memory
  openmp: -foffload-memory=pinned
  libgomp: fine-grained pinned memory allocator

Thomas Schwinge (1):
  libgomp, nvptx: Cuda pinned memory

 gcc/common.opt                                |  16 +
 gcc/coretypes.h                               |   7 +
 gcc/doc/invoke.texi                           |  16 +-
 gcc/omp-builtins.def                          |   3 +
 gcc/omp-low.cc                                |  66 ++++
 libgomp/Makefile.am                           |   2 +-
 libgomp/Makefile.in                           |   5 +-
 libgomp/allocator.c                           |  96 ++++--
 libgomp/config/gcn/allocator.c                |  17 +-
 libgomp/config/linux/allocator.c              | 234 +++++++++++++
 libgomp/config/nvptx/allocator.c              |  17 +-
 libgomp/libgomp-plugin.h                      |   2 +
 libgomp/libgomp.h                             |  14 +
 libgomp/libgomp.map                           |   1 +
 libgomp/libgomp_g.h                           |   1 +
 libgomp/omp.h.in                              |   1 +
 libgomp/omp_lib.f90.in                        |   2 +
 libgomp/plugin/plugin-nvptx.c                 |  34 ++
 libgomp/target.c                              | 136 ++++++++
 .../libgomp.c-c++-common/alloc-pinned-1.c     |  28 ++
 libgomp/testsuite/libgomp.c/alloc-pinned-1.c  | 134 ++++++++
 libgomp/testsuite/libgomp.c/alloc-pinned-2.c  | 139 ++++++++
 libgomp/testsuite/libgomp.c/alloc-pinned-3.c  | 174 ++++++++++
 libgomp/testsuite/libgomp.c/alloc-pinned-4.c  | 176 ++++++++++
 libgomp/testsuite/libgomp.c/alloc-pinned-5.c  | 128 +++++++
 libgomp/testsuite/libgomp.c/alloc-pinned-6.c  | 127 +++++++
 libgomp/testsuite/libgomp.c/alloc-pinned-7.c  |  63 ++++
 libgomp/testsuite/libgomp.c/alloc-pinned-8.c  | 127 +++++++
 .../libgomp.fortran/alloc-pinned-1.f90        |  16 +
 libgomp/usmpin-allocator.c                    | 319 ++++++++++++++++++
 30 files changed, 2051 insertions(+), 50 deletions(-)
 create mode 100644 libgomp/testsuite/libgomp.c-c++-common/alloc-pinned-1.c
 create mode 100644 libgomp/testsuite/libgomp.c/alloc-pinned-1.c
 create mode 100644 libgomp/testsuite/libgomp.c/alloc-pinned-2.c
 create mode 100644 libgomp/testsuite/libgomp.c/alloc-pinned-3.c
 create mode 100644 libgomp/testsuite/libgomp.c/alloc-pinned-4.c
 create mode 100644 libgomp/testsuite/libgomp.c/alloc-pinned-5.c
 create mode 100644 libgomp/testsuite/libgomp.c/alloc-pinned-6.c
 create mode 100644 libgomp/testsuite/libgomp.c/alloc-pinned-7.c
 create mode 100644 libgomp/testsuite/libgomp.c/alloc-pinned-8.c
 create mode 100644 libgomp/testsuite/libgomp.fortran/alloc-pinned-1.f90
 create mode 100644 libgomp/usmpin-allocator.c

-- 
2.41.0


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 1/6] libgomp: basic pinned memory on Linux
  2023-08-23 14:14 [PATCH v2 0/6] libgomp: OpenMP pinned memory omp_alloc Andrew Stubbs
@ 2023-08-23 14:14 ` Andrew Stubbs
  2023-11-22 14:26   ` Tobias Burnus
  2023-08-23 14:14 ` [PATCH v2 2/6] libgomp, openmp: Add ompx_pinned_mem_alloc Andrew Stubbs
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Andrew Stubbs @ 2023-08-23 14:14 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 2090 bytes --]


Implement the OpenMP pinned memory trait on Linux hosts using the mlock
syscall.  Pinned allocations are performed using mmap, not malloc, to ensure
that they can be unpinned safely when freed.

This implementation will work OK for page-scale allocations, and finer-grained
allocations will be implemented in a future patch.

libgomp/ChangeLog:

	* allocator.c (MEMSPACE_ALLOC): Add PIN.
	(MEMSPACE_CALLOC): Add PIN.
	(MEMSPACE_REALLOC): Add PIN.
	(MEMSPACE_FREE): Add PIN.
	(omp_init_allocator): Don't disallow the pinned trait.
	(omp_aligned_alloc): Add pinning to all MEMSPACE_* calls.
	(omp_aligned_calloc): Likewise.
	(omp_realloc): Likewise.
	(omp_free): Likewise.
	* config/linux/allocator.c: New file.
	* config/nvptx/allocator.c (MEMSPACE_ALLOC): Add PIN.
	(MEMSPACE_CALLOC): Add PIN.
	(MEMSPACE_REALLOC): Add PIN.
	(MEMSPACE_FREE): Add PIN.
	* config/gcn/allocator.c (MEMSPACE_ALLOC): Add PIN.
	(MEMSPACE_CALLOC): Add PIN.
	(MEMSPACE_REALLOC): Add PIN.
	(MEMSPACE_FREE): Add PIN.
	* testsuite/libgomp.c/alloc-pinned-1.c: New test.
	* testsuite/libgomp.c/alloc-pinned-2.c: New test.
	* testsuite/libgomp.c/alloc-pinned-3.c: New test.
	* testsuite/libgomp.c/alloc-pinned-4.c: New test.

Co-Authored-By: Thomas Schwinge <thomas@codesourcery.com>
---
 libgomp/allocator.c                          |  66 +++++----
 libgomp/config/gcn/allocator.c               |  17 +--
 libgomp/config/linux/allocator.c             |  99 +++++++++++++
 libgomp/config/nvptx/allocator.c             |  17 +--
 libgomp/testsuite/libgomp.c/alloc-pinned-1.c | 109 ++++++++++++++
 libgomp/testsuite/libgomp.c/alloc-pinned-2.c | 114 +++++++++++++++
 libgomp/testsuite/libgomp.c/alloc-pinned-3.c | 141 ++++++++++++++++++
 libgomp/testsuite/libgomp.c/alloc-pinned-4.c | 143 +++++++++++++++++++
 8 files changed, 665 insertions(+), 41 deletions(-)
 create mode 100644 libgomp/testsuite/libgomp.c/alloc-pinned-1.c
 create mode 100644 libgomp/testsuite/libgomp.c/alloc-pinned-2.c
 create mode 100644 libgomp/testsuite/libgomp.c/alloc-pinned-3.c
 create mode 100644 libgomp/testsuite/libgomp.c/alloc-pinned-4.c


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v2-0001-libgomp-basic-pinned-memory-on-Linux.patch --]
[-- Type: text/x-patch; name="v2-0001-libgomp-basic-pinned-memory-on-Linux.patch", Size: 23095 bytes --]

diff --git a/libgomp/allocator.c b/libgomp/allocator.c
index a132abc45b5..6007e64f580 100644
--- a/libgomp/allocator.c
+++ b/libgomp/allocator.c
@@ -41,20 +41,21 @@
    The following definitions (ab)use comma operators to avoid unused
    variable errors.  */
 #ifndef MEMSPACE_ALLOC
-#define MEMSPACE_ALLOC(MEMSPACE, SIZE) \
-  malloc (((void)(MEMSPACE), (SIZE)))
+#define MEMSPACE_ALLOC(MEMSPACE, SIZE, PIN) \
+  (PIN ? NULL : malloc (((void)(MEMSPACE), (SIZE))))
 #endif
 #ifndef MEMSPACE_CALLOC
-#define MEMSPACE_CALLOC(MEMSPACE, SIZE) \
-  calloc (1, (((void)(MEMSPACE), (SIZE))))
+#define MEMSPACE_CALLOC(MEMSPACE, SIZE, PIN) \
+  (PIN ? NULL : calloc (1, (((void)(MEMSPACE), (SIZE)))))
 #endif
 #ifndef MEMSPACE_REALLOC
-#define MEMSPACE_REALLOC(MEMSPACE, ADDR, OLDSIZE, SIZE) \
-  realloc (ADDR, (((void)(MEMSPACE), (void)(OLDSIZE), (SIZE))))
+#define MEMSPACE_REALLOC(MEMSPACE, ADDR, OLDSIZE, SIZE, OLDPIN, PIN) \
+   ((PIN) || (OLDPIN) ? NULL \
+   : realloc (ADDR, (((void)(MEMSPACE), (void)(OLDSIZE), (SIZE)))))
 #endif
 #ifndef MEMSPACE_FREE
-#define MEMSPACE_FREE(MEMSPACE, ADDR, SIZE) \
-  free (((void)(MEMSPACE), (void)(SIZE), (ADDR)))
+#define MEMSPACE_FREE(MEMSPACE, ADDR, SIZE, PIN) \
+  (PIN ? NULL : free (((void)(MEMSPACE), (void)(SIZE), (ADDR))))
 #endif
 #ifndef MEMSPACE_VALIDATE
 #define MEMSPACE_VALIDATE(MEMSPACE, ACCESS) \
@@ -434,10 +435,6 @@ omp_init_allocator (omp_memspace_handle_t memspace, int ntraits,
     }
 #endif
 
-  /* No support for this so far.  */
-  if (data.pinned)
-    return omp_null_allocator;
-
   ret = gomp_malloc (sizeof (struct omp_allocator_data));
   *ret = data;
 #ifndef HAVE_SYNC_BUILTINS
@@ -577,7 +574,8 @@ retry:
 	}
       else
 #endif
-	ptr = MEMSPACE_ALLOC (allocator_data->memspace, new_size);
+	ptr = MEMSPACE_ALLOC (allocator_data->memspace, new_size,
+			      allocator_data->pinned);
       if (ptr == NULL)
 	{
 #ifdef HAVE_SYNC_BUILTINS
@@ -614,7 +612,8 @@ retry:
 	  memspace = (allocator_data
 		      ? allocator_data->memspace
 		      : predefined_alloc_mapping[allocator]);
-	  ptr = MEMSPACE_ALLOC (memspace, new_size);
+	  ptr = MEMSPACE_ALLOC (memspace, new_size,
+				allocator_data && allocator_data->pinned);
 	}
       if (ptr == NULL)
 	goto fail;
@@ -646,7 +645,8 @@ fail:;
 	  || memkind
 #endif
 	  || allocator_data == NULL
-	  || allocator_data->pool_size < ~(uintptr_t) 0)
+	  || allocator_data->pool_size < ~(uintptr_t) 0
+	  || allocator_data->pinned)
 	{
 	  allocator = omp_default_mem_alloc;
 	  goto retry;
@@ -697,6 +697,7 @@ omp_free (void *ptr, omp_allocator_handle_t allocator)
 {
   struct omp_mem_header *data;
   omp_memspace_handle_t memspace = omp_default_mem_space;
+  int pinned = false;
 
   if (ptr == NULL)
     return;
@@ -738,6 +739,7 @@ omp_free (void *ptr, omp_allocator_handle_t allocator)
 #endif
 
       memspace = allocator_data->memspace;
+      pinned = allocator_data->pinned;
     }
   else
     {
@@ -762,7 +764,7 @@ omp_free (void *ptr, omp_allocator_handle_t allocator)
       memspace = predefined_alloc_mapping[data->allocator];
     }
 
-  MEMSPACE_FREE (memspace, data->ptr, data->size);
+  MEMSPACE_FREE (memspace, data->ptr, data->size, pinned);
 }
 
 ialias (omp_free)
@@ -893,7 +895,8 @@ retry:
 	}
       else
 #endif
-	ptr = MEMSPACE_CALLOC (allocator_data->memspace, new_size);
+	ptr = MEMSPACE_CALLOC (allocator_data->memspace, new_size,
+			       allocator_data->pinned);
       if (ptr == NULL)
 	{
 #ifdef HAVE_SYNC_BUILTINS
@@ -932,7 +935,8 @@ retry:
 	  memspace = (allocator_data
 		      ? allocator_data->memspace
 		      : predefined_alloc_mapping[allocator]);
-	  ptr = MEMSPACE_CALLOC (memspace, new_size);
+	  ptr = MEMSPACE_CALLOC (memspace, new_size,
+				 allocator_data && allocator_data->pinned);
 	}
       if (ptr == NULL)
 	goto fail;
@@ -964,7 +968,8 @@ fail:;
 	  || memkind
 #endif
 	  || allocator_data == NULL
-	  || allocator_data->pool_size < ~(uintptr_t) 0)
+	  || allocator_data->pool_size < ~(uintptr_t) 0
+	  || allocator_data->pinned)
 	{
 	  allocator = omp_default_mem_alloc;
 	  goto retry;
@@ -1176,9 +1181,13 @@ retry:
 #endif
       if (prev_size)
 	new_ptr = MEMSPACE_REALLOC (allocator_data->memspace, data->ptr,
-				    data->size, new_size);
+				    data->size, new_size,
+				    (free_allocator_data
+				     && free_allocator_data->pinned),
+				    allocator_data->pinned);
       else
-	new_ptr = MEMSPACE_ALLOC (allocator_data->memspace, new_size);
+	new_ptr = MEMSPACE_ALLOC (allocator_data->memspace, new_size,
+				  allocator_data->pinned);
       if (new_ptr == NULL)
 	{
 #ifdef HAVE_SYNC_BUILTINS
@@ -1231,10 +1240,14 @@ retry:
 	  memspace = (allocator_data
 		      ? allocator_data->memspace
 		      : predefined_alloc_mapping[allocator]);
-	  new_ptr = MEMSPACE_REALLOC (memspace, data->ptr, data->size, new_size);
+	  new_ptr = MEMSPACE_REALLOC (memspace, data->ptr, data->size, new_size,
+				      (free_allocator_data
+				       && free_allocator_data->pinned),
+				      allocator_data && allocator_data->pinned);
 	}
       if (new_ptr == NULL)
 	goto fail;
+
       ret = (char *) new_ptr + sizeof (struct omp_mem_header);
       ((struct omp_mem_header *) ret)[-1].ptr = new_ptr;
       ((struct omp_mem_header *) ret)[-1].size = new_size;
@@ -1264,7 +1277,8 @@ retry:
 	  memspace = (allocator_data
 		      ? allocator_data->memspace
 		      : predefined_alloc_mapping[allocator]);
-	  new_ptr = MEMSPACE_ALLOC (memspace, new_size);
+	  new_ptr = MEMSPACE_ALLOC (memspace, new_size,
+				    allocator_data && allocator_data->pinned);
 	}
       if (new_ptr == NULL)
 	goto fail;
@@ -1319,7 +1333,8 @@ retry:
     was_memspace = (free_allocator_data
 		    ? free_allocator_data->memspace
 		    : predefined_alloc_mapping[free_allocator]);
-    MEMSPACE_FREE (was_memspace, data->ptr, data->size);
+    int was_pinned = (free_allocator_data && free_allocator_data->pinned);
+    MEMSPACE_FREE (was_memspace, data->ptr, data->size, was_pinned);
   }
   return ret;
 
@@ -1337,7 +1352,8 @@ fail:;
 	  || memkind
 #endif
 	  || allocator_data == NULL
-	  || allocator_data->pool_size < ~(uintptr_t) 0)
+	  || allocator_data->pool_size < ~(uintptr_t) 0
+	  || allocator_data->pinned)
 	{
 	  allocator = omp_default_mem_alloc;
 	  goto retry;
diff --git a/libgomp/config/gcn/allocator.c b/libgomp/config/gcn/allocator.c
index 151086ea225..fa701ded0ff 100644
--- a/libgomp/config/gcn/allocator.c
+++ b/libgomp/config/gcn/allocator.c
@@ -109,14 +109,15 @@ gcn_memspace_validate (omp_memspace_handle_t memspace, unsigned access)
 	  || access != omp_atv_all);
 }
 
-#define MEMSPACE_ALLOC(MEMSPACE, SIZE) \
-  gcn_memspace_alloc (MEMSPACE, SIZE)
-#define MEMSPACE_CALLOC(MEMSPACE, SIZE) \
-  gcn_memspace_calloc (MEMSPACE, SIZE)
-#define MEMSPACE_REALLOC(MEMSPACE, ADDR, OLDSIZE, SIZE) \
-  gcn_memspace_realloc (MEMSPACE, ADDR, OLDSIZE, SIZE)
-#define MEMSPACE_FREE(MEMSPACE, ADDR, SIZE) \
-  gcn_memspace_free (MEMSPACE, ADDR, SIZE)
+#define MEMSPACE_ALLOC(MEMSPACE, SIZE, PIN) \
+  gcn_memspace_alloc (MEMSPACE, ((void)(PIN), (SIZE)))
+#define MEMSPACE_CALLOC(MEMSPACE, SIZE, PIN) \
+  gcn_memspace_calloc (MEMSPACE, ((void)(PIN), (SIZE)))
+#define MEMSPACE_REALLOC(MEMSPACE, ADDR, OLDSIZE, SIZE, OLDPIN, PIN) \
+  gcn_memspace_realloc (MEMSPACE, ADDR, OLDSIZE, \
+			((void)(PIN), (void)(OLDPIN), (SIZE)))
+#define MEMSPACE_FREE(MEMSPACE, ADDR, SIZE, PIN) \
+  gcn_memspace_free (MEMSPACE, ADDR, ((void)(PIN), (SIZE)))
 #define MEMSPACE_VALIDATE(MEMSPACE, ACCESS) \
   gcn_memspace_validate (MEMSPACE, ACCESS)
 
diff --git a/libgomp/config/linux/allocator.c b/libgomp/config/linux/allocator.c
index 64b1b4b9623..edcde9f1e81 100644
--- a/libgomp/config/linux/allocator.c
+++ b/libgomp/config/linux/allocator.c
@@ -34,4 +34,103 @@
 #define LIBGOMP_USE_LIBNUMA
 #endif
 
+/* Implement malloc routines that can handle pinned memory on Linux.
+   
+   It's possible to use mlock on any heap memory, but using munlock is
+   problematic if there are multiple pinned allocations on the same page.
+   Tracking all that manually would be possible, but adds overhead. This may
+   be worth it if there are a lot of small allocations getting pinned, but
+   this seems less likely in a HPC application.
+
+   Instead we optimize for large pinned allocations, and use mmap to ensure
+   that two pinned allocations don't share the same page.  This also means
+   that large allocations don't pin extra pages by being poorly aligned.  */
+
+#define _GNU_SOURCE
+#include <sys/mman.h>
+#include <string.h>
+#include "libgomp.h"
+
+static void *
+linux_memspace_alloc (omp_memspace_handle_t memspace, size_t size, int pin)
+{
+  (void)memspace;
+
+  if (pin)
+    {
+      void *addr = mmap (NULL, size, PROT_READ | PROT_WRITE,
+			 MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+      if (addr == MAP_FAILED)
+	return NULL;
+
+      if (mlock (addr, size))
+	{
+	  gomp_debug (0, "libgomp: failed to pin memory (ulimit too low?)\n");
+	  munmap (addr, size);
+	  return NULL;
+	}
+
+      return addr;
+    }
+  else
+    return malloc (size);
+}
+
+static void *
+linux_memspace_calloc (omp_memspace_handle_t memspace, size_t size, int pin)
+{
+  if (pin)
+    return linux_memspace_alloc (memspace, size, pin);
+  else
+    return calloc (1, size);
+}
+
+static void
+linux_memspace_free (omp_memspace_handle_t memspace, void *addr, size_t size,
+		     int pin)
+{
+  (void)memspace;
+
+  if (pin)
+    munmap (addr, size);
+  else
+    free (addr);
+}
+
+static void *
+linux_memspace_realloc (omp_memspace_handle_t memspace, void *addr,
+			size_t oldsize, size_t size, int oldpin, int pin)
+{
+  if (oldpin && pin)
+    {
+      void *newaddr = mremap (addr, oldsize, size, MREMAP_MAYMOVE);
+      if (newaddr == MAP_FAILED)
+	return NULL;
+
+      return newaddr;
+    }
+  else if (oldpin || pin)
+    {
+      void *newaddr = linux_memspace_alloc (memspace, size, pin);
+      if (newaddr)
+	{
+	  memcpy (newaddr, addr, oldsize < size ? oldsize : size);
+	  linux_memspace_free (memspace, addr, oldsize, oldpin);
+	}
+
+      return newaddr;
+    }
+  else
+    return realloc (addr, size);
+}
+
+#define MEMSPACE_ALLOC(MEMSPACE, SIZE, PIN) \
+  linux_memspace_alloc (MEMSPACE, SIZE, PIN)
+#define MEMSPACE_CALLOC(MEMSPACE, SIZE, PIN) \
+  linux_memspace_calloc (MEMSPACE, SIZE, PIN)
+#define MEMSPACE_REALLOC(MEMSPACE, ADDR, OLDSIZE, SIZE, OLDPIN, PIN) \
+  linux_memspace_realloc (MEMSPACE, ADDR, OLDSIZE, SIZE, OLDPIN, PIN)
+#define MEMSPACE_FREE(MEMSPACE, ADDR, SIZE, PIN) \
+  linux_memspace_free (MEMSPACE, ADDR, SIZE, PIN)
+
 #include "../../allocator.c"
diff --git a/libgomp/config/nvptx/allocator.c b/libgomp/config/nvptx/allocator.c
index f19ac28d32a..607ef0a77b0 100644
--- a/libgomp/config/nvptx/allocator.c
+++ b/libgomp/config/nvptx/allocator.c
@@ -117,14 +117,15 @@ nvptx_memspace_validate (omp_memspace_handle_t memspace, unsigned access)
 	  || access != omp_atv_all);
 }
 
-#define MEMSPACE_ALLOC(MEMSPACE, SIZE) \
-  nvptx_memspace_alloc (MEMSPACE, SIZE)
-#define MEMSPACE_CALLOC(MEMSPACE, SIZE) \
-  nvptx_memspace_calloc (MEMSPACE, SIZE)
-#define MEMSPACE_REALLOC(MEMSPACE, ADDR, OLDSIZE, SIZE) \
-  nvptx_memspace_realloc (MEMSPACE, ADDR, OLDSIZE, SIZE)
-#define MEMSPACE_FREE(MEMSPACE, ADDR, SIZE) \
-  nvptx_memspace_free (MEMSPACE, ADDR, SIZE)
+#define MEMSPACE_ALLOC(MEMSPACE, SIZE, PIN) \
+  nvptx_memspace_alloc (MEMSPACE, ((void)(PIN), (SIZE)))
+#define MEMSPACE_CALLOC(MEMSPACE, SIZE, PIN) \
+  nvptx_memspace_calloc (MEMSPACE, ((void)(PIN), (SIZE)))
+#define MEMSPACE_REALLOC(MEMSPACE, ADDR, OLDSIZE, SIZE, OLDPIN, PIN) \
+  nvptx_memspace_realloc (MEMSPACE, ADDR, OLDSIZE, \
+			  ((void)(OLDPIN), (void)(PIN), (SIZE)))
+#define MEMSPACE_FREE(MEMSPACE, ADDR, SIZE, PIN) \
+  nvptx_memspace_free (MEMSPACE, ADDR, ((void)(PIN), (SIZE)))
 #define MEMSPACE_VALIDATE(MEMSPACE, ACCESS) \
   nvptx_memspace_validate (MEMSPACE, ACCESS)
 
diff --git a/libgomp/testsuite/libgomp.c/alloc-pinned-1.c b/libgomp/testsuite/libgomp.c/alloc-pinned-1.c
new file mode 100644
index 00000000000..8731090bb54
--- /dev/null
+++ b/libgomp/testsuite/libgomp.c/alloc-pinned-1.c
@@ -0,0 +1,109 @@
+/* { dg-do run } */
+
+/* { dg-xfail-run-if "Pinning not implemented on this host" { ! *-*-linux-gnu } } */
+
+/* Test that pinned memory works.  */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#ifdef __linux__
+#include <sys/types.h>
+#include <unistd.h>
+
+#include <sys/mman.h>
+#include <sys/resource.h>
+
+#define PAGE_SIZE sysconf(_SC_PAGESIZE)
+#define CHECK_SIZE(SIZE) { \
+  struct rlimit limit; \
+  if (getrlimit (RLIMIT_MEMLOCK, &limit) \
+      || limit.rlim_cur <= SIZE) \
+    fprintf (stderr, "unsufficient lockable memory; please increase ulimit\n"); \
+  }
+
+int
+get_pinned_mem ()
+{
+  int pid = getpid ();
+  char buf[100];
+  sprintf (buf, "/proc/%d/status", pid);
+
+  FILE *proc = fopen (buf, "r");
+  if (!proc)
+    abort ();
+  while (fgets (buf, 100, proc))
+    {
+      int val;
+      if (sscanf (buf, "VmLck: %d", &val))
+	{
+	  fclose (proc);
+	  return val;
+	}
+    }
+  abort ();
+}
+#else
+#define PAGE_SIZE 1 /* unknown */
+#define CHECK_SIZE(SIZE) fprintf (stderr, "OS unsupported\n");
+
+int
+get_pinned_mem ()
+{
+  return 0;
+}
+#endif
+
+static void
+verify0 (char *p, size_t s)
+{
+  for (size_t i = 0; i < s; ++i)
+    if (p[i] != 0)
+      abort ();
+}
+
+#include <omp.h>
+
+int
+main ()
+{
+  /* Allocate at least a page each time, allowing space for overhead,
+     but stay within the ulimit.  */
+  const int SIZE = PAGE_SIZE - 128;
+  CHECK_SIZE (SIZE * 5);
+
+  const omp_alloctrait_t traits[] = {
+      { omp_atk_pinned, 1 }
+  };
+  omp_allocator_handle_t allocator = omp_init_allocator (omp_default_mem_space,
+							 1, traits);
+
+  // Sanity check
+  if (get_pinned_mem () != 0)
+    abort ();
+
+  void *p = omp_alloc (SIZE, allocator);
+  if (!p)
+    abort ();
+
+  int amount = get_pinned_mem ();
+  if (amount == 0)
+    abort ();
+
+  p = omp_realloc (p, SIZE * 2, allocator, allocator);
+
+  int amount2 = get_pinned_mem ();
+  if (amount2 <= amount)
+    abort ();
+
+  /* SIZE*2 ensures that it doesn't slot into the space possibly
+     vacated by realloc.  */
+  p = omp_calloc (1, SIZE * 2, allocator);
+
+  if (get_pinned_mem () <= amount2)
+    abort ();
+
+  verify0 (p, SIZE * 2);
+
+  return 0;
+}
diff --git a/libgomp/testsuite/libgomp.c/alloc-pinned-2.c b/libgomp/testsuite/libgomp.c/alloc-pinned-2.c
new file mode 100644
index 00000000000..27d2403960d
--- /dev/null
+++ b/libgomp/testsuite/libgomp.c/alloc-pinned-2.c
@@ -0,0 +1,114 @@
+/* { dg-do run } */
+
+/* { dg-xfail-run-if "Pinning not implemented on this host" { ! *-*-linux-gnu } } */
+
+/* Test that pinned memory works (pool_size code path).  */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#ifdef __linux__
+#include <sys/types.h>
+#include <unistd.h>
+
+#include <sys/mman.h>
+#include <sys/resource.h>
+
+#define PAGE_SIZE sysconf(_SC_PAGESIZE)
+#define CHECK_SIZE(SIZE) { \
+  struct rlimit limit; \
+  if (getrlimit (RLIMIT_MEMLOCK, &limit) \
+      || limit.rlim_cur <= SIZE) \
+    fprintf (stderr, "unsufficient lockable memory; please increase ulimit\n"); \
+  }
+
+int
+get_pinned_mem ()
+{
+  int pid = getpid ();
+  char buf[100];
+  sprintf (buf, "/proc/%d/status", pid);
+
+  FILE *proc = fopen (buf, "r");
+  if (!proc)
+    abort ();
+  while (fgets (buf, 100, proc))
+    {
+      int val;
+      if (sscanf (buf, "VmLck: %d", &val))
+	{
+	  fclose (proc);
+	  return val;
+	}
+    }
+  abort ();
+}
+#else
+#define PAGE_SIZE 1 /* unknown */
+#define CHECK_SIZE(SIZE) fprintf (stderr, "OS unsupported\n");
+
+int
+get_pinned_mem ()
+{
+  return 0;
+}
+#endif
+
+static void
+verify0 (char *p, size_t s)
+{
+  for (size_t i = 0; i < s; ++i)
+    if (p[i] != 0)
+      abort ();
+}
+
+#include <omp.h>
+
+int
+main ()
+{
+  /* Allocate at least a page each time, allowing space for overhead,
+     but stay within the ulimit.  */
+  const int SIZE = PAGE_SIZE - 128;
+  CHECK_SIZE (SIZE * 5);
+
+  const omp_alloctrait_t traits[] = {
+      { omp_atk_pinned, 1 },
+      { omp_atk_pool_size, SIZE * 8 }
+  };
+  omp_allocator_handle_t allocator = omp_init_allocator (omp_default_mem_space,
+							 2, traits);
+
+  // Sanity check
+  if (get_pinned_mem () != 0)
+    abort ();
+
+  void *p = omp_alloc (SIZE, allocator);
+  if (!p)
+    abort ();
+
+  int amount = get_pinned_mem ();
+  if (amount == 0)
+    abort ();
+
+  p = omp_realloc (p, SIZE * 2, allocator, allocator);
+  if (!p)
+    abort ();
+
+  int amount2 = get_pinned_mem ();
+  if (amount2 <= amount)
+    abort ();
+
+  /* SIZE*2 ensures that it doesn't slot into the space possibly
+     vacated by realloc.  */
+  p = omp_calloc (1, SIZE * 2, allocator);
+  if (!p)
+    abort ();
+
+  if (get_pinned_mem () <= amount2)
+    abort ();
+
+  verify0 (p, SIZE * 2);
+
+  return 0;
+}
diff --git a/libgomp/testsuite/libgomp.c/alloc-pinned-3.c b/libgomp/testsuite/libgomp.c/alloc-pinned-3.c
new file mode 100644
index 00000000000..6521f135c46
--- /dev/null
+++ b/libgomp/testsuite/libgomp.c/alloc-pinned-3.c
@@ -0,0 +1,141 @@
+/* { dg-do run } */
+
+/* Test that pinned memory fails correctly.  */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#ifdef __linux__
+#include <sys/types.h>
+#include <unistd.h>
+
+#include <sys/mman.h>
+#include <sys/resource.h>
+
+#define PAGE_SIZE sysconf(_SC_PAGESIZE)
+
+int
+get_pinned_mem ()
+{
+  int pid = getpid ();
+  char buf[100];
+  sprintf (buf, "/proc/%d/status", pid);
+
+  FILE *proc = fopen (buf, "r");
+  if (!proc)
+    abort ();
+  while (fgets (buf, 100, proc))
+    {
+      int val;
+      if (sscanf (buf, "VmLck: %d", &val))
+	{
+	  fclose (proc);
+	  return val;
+	}
+    }
+  abort ();
+}
+
+void
+set_pin_limit (int size)
+{
+  struct rlimit limit;
+  if (getrlimit (RLIMIT_MEMLOCK, &limit))
+    abort ();
+  limit.rlim_cur = (limit.rlim_max < size ? limit.rlim_max : size);
+  if (setrlimit (RLIMIT_MEMLOCK, &limit))
+    abort ();
+}
+#else
+int
+#define PAGE_SIZE 10000 * 1024 /* unknown */
+
+get_pinned_mem ()
+{
+  return 0;
+}
+
+void
+set_pin_limit ()
+{
+}
+#endif
+
+static void
+verify0 (char *p, size_t s)
+{
+  for (size_t i = 0; i < s; ++i)
+    if (p[i] != 0)
+      abort ();
+}
+
+#include <omp.h>
+
+int
+main ()
+{
+  /* This needs to be large enough to cover multiple pages.  */
+  const int SIZE = PAGE_SIZE * 4;
+
+  /* Pinned memory, no fallback.  */
+  const omp_alloctrait_t traits1[] = {
+      { omp_atk_pinned, 1 },
+      { omp_atk_fallback, omp_atv_null_fb }
+  };
+  omp_allocator_handle_t allocator1 = omp_init_allocator (omp_default_mem_space,
+							  2, traits1);
+
+  /* Pinned memory, plain memory fallback.  */
+  const omp_alloctrait_t traits2[] = {
+      { omp_atk_pinned, 1 },
+      { omp_atk_fallback, omp_atv_default_mem_fb }
+  };
+  omp_allocator_handle_t allocator2 = omp_init_allocator (omp_default_mem_space,
+							  2, traits2);
+
+  /* Ensure that the limit is smaller than the allocation.  */
+  set_pin_limit (SIZE / 2);
+
+  // Sanity check
+  if (get_pinned_mem () != 0)
+    abort ();
+
+  // Should fail
+  void *p = omp_alloc (SIZE, allocator1);
+  if (p)
+    abort ();
+
+  // Should fail
+  p = omp_calloc (1, SIZE, allocator1);
+  if (p)
+    abort ();
+
+  // Should fall back
+  p = omp_alloc (SIZE, allocator2);
+  if (!p)
+    abort ();
+
+  // Should fall back
+  p = omp_calloc (1, SIZE, allocator2);
+  if (!p)
+    abort ();
+  verify0 (p, SIZE);
+
+  // Should fail to realloc
+  void *notpinned = omp_alloc (SIZE, omp_default_mem_alloc);
+  p = omp_realloc (notpinned, SIZE, allocator1, omp_default_mem_alloc);
+  if (!notpinned || p)
+    abort ();
+
+  // Should fall back to no realloc needed
+  p = omp_realloc (notpinned, SIZE, allocator2, omp_default_mem_alloc);
+  if (p != notpinned)
+    abort ();
+
+  // No memory should have been pinned
+  int amount = get_pinned_mem ();
+  if (amount != 0)
+    abort ();
+
+  return 0;
+}
diff --git a/libgomp/testsuite/libgomp.c/alloc-pinned-4.c b/libgomp/testsuite/libgomp.c/alloc-pinned-4.c
new file mode 100644
index 00000000000..51917557c91
--- /dev/null
+++ b/libgomp/testsuite/libgomp.c/alloc-pinned-4.c
@@ -0,0 +1,143 @@
+/* { dg-do run } */
+
+/* Test that pinned memory fails correctly, pool_size code path.  */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#ifdef __linux__
+#include <sys/types.h>
+#include <unistd.h>
+
+#include <sys/mman.h>
+#include <sys/resource.h>
+
+#define PAGE_SIZE sysconf(_SC_PAGESIZE)
+
+int
+get_pinned_mem ()
+{
+  int pid = getpid ();
+  char buf[100];
+  sprintf (buf, "/proc/%d/status", pid);
+
+  FILE *proc = fopen (buf, "r");
+  if (!proc)
+    abort ();
+  while (fgets (buf, 100, proc))
+    {
+      int val;
+      if (sscanf (buf, "VmLck: %d", &val))
+	{
+	  fclose (proc);
+	  return val;
+	}
+    }
+  abort ();
+}
+
+void
+set_pin_limit (int size)
+{
+  struct rlimit limit;
+  if (getrlimit (RLIMIT_MEMLOCK, &limit))
+    abort ();
+  limit.rlim_cur = (limit.rlim_max < size ? limit.rlim_max : size);
+  if (setrlimit (RLIMIT_MEMLOCK, &limit))
+    abort ();
+}
+#else
+int
+#define PAGE_SIZE 10000 * 1024 /* unknown */
+
+get_pinned_mem ()
+{
+  return 0;
+}
+
+void
+set_pin_limit ()
+{
+}
+#endif
+
+static void
+verify0 (char *p, size_t s)
+{
+  for (size_t i = 0; i < s; ++i)
+    if (p[i] != 0)
+      abort ();
+}
+
+#include <omp.h>
+
+int
+main ()
+{
+  /* This needs to be large enough to cover multiple pages.  */
+  const int SIZE = PAGE_SIZE * 4;
+
+  /* Pinned memory, no fallback.  */
+  const omp_alloctrait_t traits1[] = {
+      { omp_atk_pinned, 1 },
+      { omp_atk_fallback, omp_atv_null_fb },
+      { omp_atk_pool_size, SIZE * 8 }
+  };
+  omp_allocator_handle_t allocator1 = omp_init_allocator (omp_default_mem_space,
+							  3, traits1);
+
+  /* Pinned memory, plain memory fallback.  */
+  const omp_alloctrait_t traits2[] = {
+      { omp_atk_pinned, 1 },
+      { omp_atk_fallback, omp_atv_default_mem_fb },
+      { omp_atk_pool_size, SIZE * 8 }
+  };
+  omp_allocator_handle_t allocator2 = omp_init_allocator (omp_default_mem_space,
+							  3, traits2);
+
+  /* Ensure that the limit is smaller than the allocation.  */
+  set_pin_limit (SIZE / 2);
+
+  // Sanity check
+  if (get_pinned_mem () != 0)
+    abort ();
+
+  // Should fail
+  void *p = omp_alloc (SIZE, allocator1);
+  if (p)
+    abort ();
+
+  // Should fail
+  p = omp_calloc (1, SIZE, allocator1);
+  if (p)
+    abort ();
+
+  // Should fall back
+  p = omp_alloc (SIZE, allocator2);
+  if (!p)
+    abort ();
+
+  // Should fall back
+  p = omp_calloc (1, SIZE, allocator2);
+  if (!p)
+    abort ();
+  verify0 (p, SIZE);
+
+  // Should fail to realloc
+  void *notpinned = omp_alloc (SIZE, omp_default_mem_alloc);
+  p = omp_realloc (notpinned, SIZE, allocator1, omp_default_mem_alloc);
+  if (!notpinned || p)
+    abort ();
+
+  // Should fall back to no realloc needed
+  p = omp_realloc (notpinned, SIZE, allocator2, omp_default_mem_alloc);
+  if (p != notpinned)
+    abort ();
+
+  // No memory should have been pinned
+  int amount = get_pinned_mem ();
+  if (amount != 0)
+    abort ();
+
+  return 0;
+}

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 2/6] libgomp, openmp: Add ompx_pinned_mem_alloc
  2023-08-23 14:14 [PATCH v2 0/6] libgomp: OpenMP pinned memory omp_alloc Andrew Stubbs
  2023-08-23 14:14 ` [PATCH v2 1/6] libgomp: basic pinned memory on Linux Andrew Stubbs
@ 2023-08-23 14:14 ` Andrew Stubbs
  2023-08-23 14:14 ` [PATCH v2 3/6] openmp: Add -foffload-memory Andrew Stubbs
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Andrew Stubbs @ 2023-08-23 14:14 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1570 bytes --]


This creates a new predefined allocator as a shortcut for using pinned
memory with OpenMP.  The name uses the OpenMP extension space and is
intended to be consistent with other OpenMP implementations currently in
development.

The allocator is equivalent to using a custom allocator with the pinned
trait and the null fallback trait.

libgomp/ChangeLog:

	* allocator.c (omp_max_predefined_alloc): Update.
	(predefined_alloc_mapping): Add ompx_pinned_mem_alloc entry.
	(omp_aligned_alloc): Support ompx_pinned_mem_alloc.
	(omp_free): Likewise.
	(omp_aligned_calloc): Likewise.
	(omp_realloc): Likewise.
	* omp.h.in (omp_allocator_handle_t): Add ompx_pinned_mem_alloc.
	* omp_lib.f90.in: Add ompx_pinned_mem_alloc.
	* testsuite/libgomp.c/alloc-pinned-5.c: New test.
	* testsuite/libgomp.c/alloc-pinned-6.c: New test.
	* testsuite/libgomp.fortran/alloc-pinned-1.f90: New test.

Co-Authored-By: Thomas Schwinge <thomas@codesourcery.com>
---
 libgomp/allocator.c                           |  58 ++++++----
 libgomp/omp.h.in                              |   1 +
 libgomp/omp_lib.f90.in                        |   2 +
 libgomp/testsuite/libgomp.c/alloc-pinned-5.c  | 103 ++++++++++++++++++
 libgomp/testsuite/libgomp.c/alloc-pinned-6.c  | 101 +++++++++++++++++
 .../libgomp.fortran/alloc-pinned-1.f90        |  16 +++
 6 files changed, 262 insertions(+), 19 deletions(-)
 create mode 100644 libgomp/testsuite/libgomp.c/alloc-pinned-5.c
 create mode 100644 libgomp/testsuite/libgomp.c/alloc-pinned-6.c
 create mode 100644 libgomp/testsuite/libgomp.fortran/alloc-pinned-1.f90


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v2-0002-libgomp-openmp-Add-ompx_pinned_mem_alloc.patch --]
[-- Type: text/x-patch; name="v2-0002-libgomp-openmp-Add-ompx_pinned_mem_alloc.patch", Size: 11316 bytes --]

diff --git a/libgomp/allocator.c b/libgomp/allocator.c
index 6007e64f580..39ba1d07bc7 100644
--- a/libgomp/allocator.c
+++ b/libgomp/allocator.c
@@ -35,7 +35,7 @@
 #include <dlfcn.h>
 #endif
 
-#define omp_max_predefined_alloc omp_thread_mem_alloc
+#define omp_max_predefined_alloc ompx_pinned_mem_alloc
 
 /* These macros may be overridden in config/<target>/allocator.c.
    The following definitions (ab)use comma operators to avoid unused
@@ -76,6 +76,7 @@ static const omp_memspace_handle_t predefined_alloc_mapping[] = {
   omp_low_lat_mem_space,   /* omp_cgroup_mem_alloc. */
   omp_low_lat_mem_space,   /* omp_pteam_mem_alloc. */
   omp_low_lat_mem_space,   /* omp_thread_mem_alloc. */
+  omp_default_mem_space,   /* ompx_pinned_mem_alloc. */
 };
 
 enum gomp_numa_memkind_kind
@@ -612,8 +613,10 @@ retry:
 	  memspace = (allocator_data
 		      ? allocator_data->memspace
 		      : predefined_alloc_mapping[allocator]);
-	  ptr = MEMSPACE_ALLOC (memspace, new_size,
-				allocator_data && allocator_data->pinned);
+	  int pinned = (allocator_data
+			? allocator_data->pinned
+			: allocator == ompx_pinned_mem_alloc);
+	  ptr = MEMSPACE_ALLOC (memspace, new_size, pinned);
 	}
       if (ptr == NULL)
 	goto fail;
@@ -634,7 +637,8 @@ retry:
 fail:;
   int fallback = (allocator_data
 		  ? allocator_data->fallback
-		  : allocator == omp_default_mem_alloc
+		  : (allocator == omp_default_mem_alloc
+		     || allocator == ompx_pinned_mem_alloc)
 		  ? omp_atv_null_fb
 		  : omp_atv_default_mem_fb);
   switch (fallback)
@@ -762,6 +766,7 @@ omp_free (void *ptr, omp_allocator_handle_t allocator)
 #endif
 
       memspace = predefined_alloc_mapping[data->allocator];
+      pinned = (data->allocator == ompx_pinned_mem_alloc);
     }
 
   MEMSPACE_FREE (memspace, data->ptr, data->size, pinned);
@@ -935,8 +940,10 @@ retry:
 	  memspace = (allocator_data
 		      ? allocator_data->memspace
 		      : predefined_alloc_mapping[allocator]);
-	  ptr = MEMSPACE_CALLOC (memspace, new_size,
-				 allocator_data && allocator_data->pinned);
+	  int pinned = (allocator_data
+			? allocator_data->pinned
+			: allocator == ompx_pinned_mem_alloc);
+	  ptr = MEMSPACE_CALLOC (memspace, new_size, pinned);
 	}
       if (ptr == NULL)
 	goto fail;
@@ -957,7 +964,8 @@ retry:
 fail:;
   int fallback = (allocator_data
 		  ? allocator_data->fallback
-		  : allocator == omp_default_mem_alloc
+		  : (allocator == omp_default_mem_alloc
+		     || allocator == ompx_pinned_mem_alloc)
 		  ? omp_atv_null_fb
 		  : omp_atv_default_mem_fb);
   switch (fallback)
@@ -1180,11 +1188,14 @@ retry:
       else
 #endif
       if (prev_size)
-	new_ptr = MEMSPACE_REALLOC (allocator_data->memspace, data->ptr,
-				    data->size, new_size,
-				    (free_allocator_data
-				     && free_allocator_data->pinned),
-				    allocator_data->pinned);
+	{
+	  int was_pinned = (free_allocator_data
+			    ? free_allocator_data->pinned
+			    : free_allocator == ompx_pinned_mem_alloc);
+	  new_ptr = MEMSPACE_REALLOC (allocator_data->memspace, data->ptr,
+				      data->size, new_size, was_pinned,
+				      allocator_data->pinned);
+	}
       else
 	new_ptr = MEMSPACE_ALLOC (allocator_data->memspace, new_size,
 				  allocator_data->pinned);
@@ -1240,10 +1251,14 @@ retry:
 	  memspace = (allocator_data
 		      ? allocator_data->memspace
 		      : predefined_alloc_mapping[allocator]);
+	  int was_pinned = (free_allocator_data
+			    ? free_allocator_data->pinned
+			    : free_allocator == ompx_pinned_mem_alloc);
+	  int pinned = (allocator_data
+			? allocator_data->pinned
+			: allocator == ompx_pinned_mem_alloc);
 	  new_ptr = MEMSPACE_REALLOC (memspace, data->ptr, data->size, new_size,
-				      (free_allocator_data
-				       && free_allocator_data->pinned),
-				      allocator_data && allocator_data->pinned);
+				      was_pinned, pinned);
 	}
       if (new_ptr == NULL)
 	goto fail;
@@ -1277,8 +1292,10 @@ retry:
 	  memspace = (allocator_data
 		      ? allocator_data->memspace
 		      : predefined_alloc_mapping[allocator]);
-	  new_ptr = MEMSPACE_ALLOC (memspace, new_size,
-				    allocator_data && allocator_data->pinned);
+	  int pinned = (allocator_data
+			? allocator_data->pinned
+			: allocator == ompx_pinned_mem_alloc);
+	  new_ptr = MEMSPACE_ALLOC (memspace, new_size, pinned);
 	}
       if (new_ptr == NULL)
 	goto fail;
@@ -1333,7 +1350,9 @@ retry:
     was_memspace = (free_allocator_data
 		    ? free_allocator_data->memspace
 		    : predefined_alloc_mapping[free_allocator]);
-    int was_pinned = (free_allocator_data && free_allocator_data->pinned);
+    int was_pinned = (free_allocator_data
+		      ? free_allocator_data->pinned
+		      : free_allocator == ompx_pinned_mem_alloc);
     MEMSPACE_FREE (was_memspace, data->ptr, data->size, was_pinned);
   }
   return ret;
@@ -1341,7 +1360,8 @@ retry:
 fail:;
   int fallback = (allocator_data
 		  ? allocator_data->fallback
-		  : allocator == omp_default_mem_alloc
+		  : (allocator == omp_default_mem_alloc
+		     || allocator == ompx_pinned_mem_alloc)
 		  ? omp_atv_null_fb
 		  : omp_atv_default_mem_fb);
   switch (fallback)
diff --git a/libgomp/omp.h.in b/libgomp/omp.h.in
index bd1286c2a3f..66989e693b7 100644
--- a/libgomp/omp.h.in
+++ b/libgomp/omp.h.in
@@ -134,6 +134,7 @@ typedef enum omp_allocator_handle_t __GOMP_UINTPTR_T_ENUM
   omp_cgroup_mem_alloc = 6,
   omp_pteam_mem_alloc = 7,
   omp_thread_mem_alloc = 8,
+  ompx_pinned_mem_alloc = 9,
   __omp_allocator_handle_t_max__ = __UINTPTR_MAX__
 } omp_allocator_handle_t;
 
diff --git a/libgomp/omp_lib.f90.in b/libgomp/omp_lib.f90.in
index e4515271252..699f97053de 100644
--- a/libgomp/omp_lib.f90.in
+++ b/libgomp/omp_lib.f90.in
@@ -158,6 +158,8 @@
                  parameter :: omp_pteam_mem_alloc = 7
         integer (kind=omp_allocator_handle_kind), &
                  parameter :: omp_thread_mem_alloc = 8
+        integer (kind=omp_allocator_handle_kind), &
+                 parameter :: ompx_pinned_mem_alloc = 9
         integer (omp_memspace_handle_kind), &
                  parameter :: omp_default_mem_space = 0
         integer (omp_memspace_handle_kind), &
diff --git a/libgomp/testsuite/libgomp.c/alloc-pinned-5.c b/libgomp/testsuite/libgomp.c/alloc-pinned-5.c
new file mode 100644
index 00000000000..9c69dbb7cde
--- /dev/null
+++ b/libgomp/testsuite/libgomp.c/alloc-pinned-5.c
@@ -0,0 +1,103 @@
+/* { dg-do run } */
+
+/* { dg-xfail-run-if "Pinning not implemented on this host" { ! *-*-linux-gnu } } */
+
+/* Test that ompx_pinned_mem_alloc works.  */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#ifdef __linux__
+#include <sys/types.h>
+#include <unistd.h>
+
+#include <sys/mman.h>
+#include <sys/resource.h>
+
+#define PAGE_SIZE sysconf(_SC_PAGESIZE)
+#define CHECK_SIZE(SIZE) { \
+  struct rlimit limit; \
+  if (getrlimit (RLIMIT_MEMLOCK, &limit) \
+      || limit.rlim_cur <= SIZE) \
+    fprintf (stderr, "unsufficient lockable memory; please increase ulimit\n"); \
+  }
+
+int
+get_pinned_mem ()
+{
+  int pid = getpid ();
+  char buf[100];
+  sprintf (buf, "/proc/%d/status", pid);
+
+  FILE *proc = fopen (buf, "r");
+  if (!proc)
+    abort ();
+  while (fgets (buf, 100, proc))
+    {
+      int val;
+      if (sscanf (buf, "VmLck: %d", &val))
+	{
+	  fclose (proc);
+	  return val;
+	}
+    }
+  abort ();
+}
+#else
+#define PAGE_SIZE 1 /* unknown */
+#define CHECK_SIZE(SIZE) fprintf (stderr, "OS unsupported\n");
+
+int
+get_pinned_mem ()
+{
+  return 0;
+}
+#endif
+
+static void
+verify0 (char *p, size_t s)
+{
+  for (size_t i = 0; i < s; ++i)
+    if (p[i] != 0)
+      abort ();
+}
+
+#include <omp.h>
+
+int
+main ()
+{
+  /* Allocate at least a page each time, allowing space for overhead,
+     but stay within the ulimit.  */
+  const int SIZE = PAGE_SIZE - 128;
+  CHECK_SIZE (SIZE * 5);
+
+  // Sanity check
+  if (get_pinned_mem () != 0)
+    abort ();
+
+  void *p = omp_alloc (SIZE, ompx_pinned_mem_alloc);
+  if (!p)
+    abort ();
+
+  int amount = get_pinned_mem ();
+  if (amount == 0)
+    abort ();
+
+  p = omp_realloc (p, SIZE * 2, ompx_pinned_mem_alloc, ompx_pinned_mem_alloc);
+
+  int amount2 = get_pinned_mem ();
+  if (amount2 <= amount)
+    abort ();
+
+  /* SIZE*2 ensures that it doesn't slot into the space possibly
+     vacated by realloc.  */
+  p = omp_calloc (1, SIZE * 2, ompx_pinned_mem_alloc);
+
+  if (get_pinned_mem () <= amount2)
+    abort ();
+
+  verify0 (p, SIZE * 2);
+
+  return 0;
+}
diff --git a/libgomp/testsuite/libgomp.c/alloc-pinned-6.c b/libgomp/testsuite/libgomp.c/alloc-pinned-6.c
new file mode 100644
index 00000000000..f80a0264f97
--- /dev/null
+++ b/libgomp/testsuite/libgomp.c/alloc-pinned-6.c
@@ -0,0 +1,101 @@
+/* { dg-do run } */
+
+/* Test that ompx_pinned_mem_alloc fails correctly.  */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#ifdef __linux__
+#include <sys/types.h>
+#include <unistd.h>
+
+#include <sys/mman.h>
+#include <sys/resource.h>
+
+#define PAGE_SIZE sysconf(_SC_PAGESIZE)
+
+int
+get_pinned_mem ()
+{
+  int pid = getpid ();
+  char buf[100];
+  sprintf (buf, "/proc/%d/status", pid);
+
+  FILE *proc = fopen (buf, "r");
+  if (!proc)
+    abort ();
+  while (fgets (buf, 100, proc))
+    {
+      int val;
+      if (sscanf (buf, "VmLck: %d", &val))
+	{
+	  fclose (proc);
+	  return val;
+	}
+    }
+  abort ();
+}
+
+void
+set_pin_limit (int size)
+{
+  struct rlimit limit;
+  if (getrlimit (RLIMIT_MEMLOCK, &limit))
+    abort ();
+  limit.rlim_cur = (limit.rlim_max < size ? limit.rlim_max : size);
+  if (setrlimit (RLIMIT_MEMLOCK, &limit))
+    abort ();
+}
+#else
+#define PAGE_SIZE 10000 * 1024 /* unknown */
+
+int
+get_pinned_mem ()
+{
+  return 0;
+}
+
+void
+set_pin_limit ()
+{
+}
+#endif
+
+#include <omp.h>
+
+int
+main ()
+{
+  /* Allocate at least a page each time, but stay within the ulimit.  */
+  const int SIZE = PAGE_SIZE * 4;
+
+  /* Ensure that the limit is smaller than the allocation.  */
+  set_pin_limit (SIZE / 2);
+
+  // Sanity check
+  if (get_pinned_mem () != 0)
+    abort ();
+
+  // Should fail
+  void *p = omp_alloc (SIZE, ompx_pinned_mem_alloc);
+  if (p)
+    abort ();
+
+  // Should fail
+  p = omp_calloc (1, SIZE, ompx_pinned_mem_alloc);
+  if (p)
+    abort ();
+
+  // Should fail to realloc
+  void *notpinned = omp_alloc (SIZE, omp_default_mem_alloc);
+  p = omp_realloc (notpinned, SIZE, ompx_pinned_mem_alloc, omp_default_mem_alloc);
+  if (!notpinned || p)
+    abort ();
+
+  // No memory should have been pinned
+  int amount = get_pinned_mem ();
+  if (amount != 0)
+    abort ();
+
+  return 0;
+}
diff --git a/libgomp/testsuite/libgomp.fortran/alloc-pinned-1.f90 b/libgomp/testsuite/libgomp.fortran/alloc-pinned-1.f90
new file mode 100644
index 00000000000..798dc3d5a12
--- /dev/null
+++ b/libgomp/testsuite/libgomp.fortran/alloc-pinned-1.f90
@@ -0,0 +1,16 @@
+! Ensure that the ompx_pinned_mem_alloc predefined allocator is present and
+! accepted.  The majority of the functionality testing lives in the C tests.
+!
+! { dg-xfail-run-if "Pinning not implemented on this host" { ! *-*-linux-gnu } }
+
+program main
+  use omp_lib
+  use ISO_C_Binding
+  implicit none (external, type)
+
+  type(c_ptr) :: p
+
+  p = omp_alloc (10_c_size_t, ompx_pinned_mem_alloc);
+  if (.not. c_associated (p)) stop 1
+  call omp_free (p, ompx_pinned_mem_alloc);
+end program main

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 3/6] openmp: Add -foffload-memory
  2023-08-23 14:14 [PATCH v2 0/6] libgomp: OpenMP pinned memory omp_alloc Andrew Stubbs
  2023-08-23 14:14 ` [PATCH v2 1/6] libgomp: basic pinned memory on Linux Andrew Stubbs
  2023-08-23 14:14 ` [PATCH v2 2/6] libgomp, openmp: Add ompx_pinned_mem_alloc Andrew Stubbs
@ 2023-08-23 14:14 ` Andrew Stubbs
  2023-08-23 14:14 ` [PATCH v2 4/6] openmp: -foffload-memory=pinned Andrew Stubbs
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Andrew Stubbs @ 2023-08-23 14:14 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 409 bytes --]


Add a new option.  It's inactive until I add some follow-up patches.

gcc/ChangeLog:

	* common.opt: Add -foffload-memory and its enum values.
	* coretypes.h (enum offload_memory): New.
	* doc/invoke.texi: Document -foffload-memory.
---
 gcc/common.opt      | 16 ++++++++++++++++
 gcc/coretypes.h     |  7 +++++++
 gcc/doc/invoke.texi | 16 +++++++++++++++-
 3 files changed, 38 insertions(+), 1 deletion(-)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v2-0003-openmp-Add-foffload-memory.patch --]
[-- Type: text/x-patch; name="v2-0003-openmp-Add-foffload-memory.patch", Size: 2914 bytes --]

diff --git a/gcc/common.opt b/gcc/common.opt
index 0888c15b88f..541213d285f 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -2243,6 +2243,22 @@ Enum(offload_abi) String(ilp32) Value(OFFLOAD_ABI_ILP32)
 EnumValue
 Enum(offload_abi) String(lp64) Value(OFFLOAD_ABI_LP64)
 
+foffload-memory=
+Common Joined RejectNegative Enum(offload_memory) Var(flag_offload_memory) Init(OFFLOAD_MEMORY_NONE)
+-foffload-memory=[none|unified|pinned]	Use an offload memory optimization.
+
+Enum
+Name(offload_memory) Type(enum offload_memory) UnknownError(Unknown offload memory option %qs)
+
+EnumValue
+Enum(offload_memory) String(none) Value(OFFLOAD_MEMORY_NONE)
+
+EnumValue
+Enum(offload_memory) String(unified) Value(OFFLOAD_MEMORY_UNIFIED)
+
+EnumValue
+Enum(offload_memory) String(pinned) Value(OFFLOAD_MEMORY_PINNED)
+
 fomit-frame-pointer
 Common Var(flag_omit_frame_pointer) Optimization
 When possible do not generate stack frames.
diff --git a/gcc/coretypes.h b/gcc/coretypes.h
index 3e9a2f19e27..a7feed51b0b 100644
--- a/gcc/coretypes.h
+++ b/gcc/coretypes.h
@@ -207,6 +207,13 @@ enum offload_abi {
   OFFLOAD_ABI_ILP32
 };
 
+/* Types of memory optimization for an offload device.  */
+enum offload_memory {
+  OFFLOAD_MEMORY_NONE,
+  OFFLOAD_MEMORY_UNIFIED,
+  OFFLOAD_MEMORY_PINNED
+};
+
 /* Types of profile update methods.  */
 enum profile_update {
   PROFILE_UPDATE_SINGLE,
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index ef3f4098986..be780dc41d8 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -202,7 +202,7 @@ in the following sections.
 -fno-builtin  -fno-builtin-@var{function}  -fcond-mismatch
 -ffreestanding  -fgimple  -fgnu-tm  -fgnu89-inline  -fhosted
 -flax-vector-conversions  -fms-extensions
--foffload=@var{arg}  -foffload-options=@var{arg}
+-foffload=@var{arg}  -foffload-options=@var{arg} -foffload-memory=@var{arg} 
 -fopenacc  -fopenacc-dim=@var{geom}
 -fopenmp  -fopenmp-simd  -fopenmp-target-simd-clone@r{[}=@var{device-type}@r{]}
 -fpermitted-flt-eval-methods=@var{standard}
@@ -2740,6 +2740,20 @@ Typical command lines are
 -foffload-options=amdgcn-amdhsa=-march=gfx906
 @end smallexample
 
+@opindex foffload-memory
+@cindex OpenMP offloading memory modes
+@item -foffload-memory=none
+@itemx -foffload-memory=unified
+@itemx -foffload-memory=pinned
+Enable a memory optimization mode to use with OpenMP.  The default behavior,
+@option{-foffload-memory=none}, is to do nothing special (unless enabled via
+a requires directive in the code).  @option{-foffload-memory=unified} is
+equivalent to @code{#pragma omp requires unified_shared_memory}.
+@option{-foffload-memory=pinned} forces all host memory to be pinned (this
+mode may require the user to increase the ulimit setting for locked memory).
+All translation units must select the same setting to avoid undefined
+behavior.
+
 @opindex fopenacc
 @cindex OpenACC accelerator programming
 @item -fopenacc

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 4/6] openmp: -foffload-memory=pinned
  2023-08-23 14:14 [PATCH v2 0/6] libgomp: OpenMP pinned memory omp_alloc Andrew Stubbs
                   ` (2 preceding siblings ...)
  2023-08-23 14:14 ` [PATCH v2 3/6] openmp: Add -foffload-memory Andrew Stubbs
@ 2023-08-23 14:14 ` Andrew Stubbs
  2023-08-23 14:14 ` [PATCH v2 5/6] libgomp, nvptx: Cuda pinned memory Andrew Stubbs
  2023-08-23 14:14 ` [PATCH v2 6/6] libgomp: fine-grained pinned memory allocator Andrew Stubbs
  5 siblings, 0 replies; 15+ messages in thread
From: Andrew Stubbs @ 2023-08-23 14:14 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 1745 bytes --]


Implement the -foffload-memory=pinned option such that libgomp is
instructed to enable fully-pinned memory at start-up.  The option is
intended to provide a performance boost to certain offload programs without
modifying the code.

This feature only works on Linux, at present, and simply calls mlockall to
enable always-on memory pinning.  It requires that the ulimit feature is
set high enough to accommodate all the program's memory usage.

In this mode the ompx_pinned_memory_alloc feature is disabled as it is not
needed and may conflict.

gcc/ChangeLog:

	* omp-builtins.def (BUILT_IN_GOMP_ENABLE_PINNED_MODE): New.
	* omp-low.cc (omp_enable_pinned_mode): New function.
	(execute_lower_omp): Call omp_enable_pinned_mode.

libgomp/ChangeLog:

	* config/linux/allocator.c (always_pinned_mode): New variable.
	(GOMP_enable_pinned_mode): New function.
	(linux_memspace_alloc): Disable pinning when always_pinned_mode set.
	(linux_memspace_calloc): Likewise.
	(linux_memspace_free): Likewise.
	(linux_memspace_realloc): Likewise.
	* libgomp.map: Add GOMP_enable_pinned_mode.
	* testsuite/libgomp.c/alloc-pinned-7.c: New test.
	* testsuite/libgomp.c-c++-common/alloc-pinned-1.c: New test.
---
 gcc/omp-builtins.def                          |  3 +
 gcc/omp-low.cc                                | 66 +++++++++++++++++++
 libgomp/config/linux/allocator.c              | 26 ++++++++
 libgomp/libgomp.map                           |  1 +
 .../libgomp.c-c++-common/alloc-pinned-1.c     | 28 ++++++++
 libgomp/testsuite/libgomp.c/alloc-pinned-7.c  | 63 ++++++++++++++++++
 6 files changed, 187 insertions(+)
 create mode 100644 libgomp/testsuite/libgomp.c-c++-common/alloc-pinned-1.c
 create mode 100644 libgomp/testsuite/libgomp.c/alloc-pinned-7.c


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v2-0004-openmp-foffload-memory-pinned.patch --]
[-- Type: text/x-patch; name="v2-0004-openmp-foffload-memory-pinned.patch", Size: 7159 bytes --]

diff --git a/gcc/omp-builtins.def b/gcc/omp-builtins.def
index e0f03263db0..1f7280f6b36 100644
--- a/gcc/omp-builtins.def
+++ b/gcc/omp-builtins.def
@@ -470,3 +470,6 @@ DEF_GOMP_BUILTIN (BUILT_IN_GOMP_WARNING, "GOMP_warning",
 		  BT_FN_VOID_CONST_PTR_SIZE, ATTR_NOTHROW_LEAF_LIST)
 DEF_GOMP_BUILTIN (BUILT_IN_GOMP_ERROR, "GOMP_error",
 		  BT_FN_VOID_CONST_PTR_SIZE, ATTR_COLD_NORETURN_NOTHROW_LEAF_LIST)
+DEF_GOMP_BUILTIN (BUILT_IN_GOMP_ENABLE_PINNED_MODE,
+		  "GOMP_enable_pinned_mode",
+		  BT_FN_VOID, ATTR_NOTHROW_LIST)
diff --git a/gcc/omp-low.cc b/gcc/omp-low.cc
index b882df048ef..64bc0662332 100644
--- a/gcc/omp-low.cc
+++ b/gcc/omp-low.cc
@@ -14683,6 +14683,68 @@ lower_omp (gimple_seq *body, omp_context *ctx)
   input_location = saved_location;
 }
 
+/* Emit a constructor function to enable -foffload-memory=pinned
+   at runtime.  Libgomp handles the OS mode setting, but we need to trigger
+   it by calling GOMP_enable_pinned mode before the program proper runs.  */
+
+static void
+omp_enable_pinned_mode ()
+{
+  static bool visited = false;
+  if (visited)
+    return;
+  visited = true;
+
+  /* Create a new function like this:
+     
+       static void __attribute__((constructor))
+       __set_pinned_mode ()
+       {
+         GOMP_enable_pinned_mode ();
+       }
+  */
+
+  tree name = get_identifier ("__set_pinned_mode");
+  tree voidfntype = build_function_type_list (void_type_node, NULL_TREE);
+  tree decl = build_decl (UNKNOWN_LOCATION, FUNCTION_DECL, name, voidfntype);
+
+  TREE_STATIC (decl) = 1;
+  TREE_USED (decl) = 1;
+  DECL_ARTIFICIAL (decl) = 1;
+  DECL_IGNORED_P (decl) = 0;
+  TREE_PUBLIC (decl) = 0;
+  DECL_UNINLINABLE (decl) = 1;
+  DECL_EXTERNAL (decl) = 0;
+  DECL_CONTEXT (decl) = NULL_TREE;
+  DECL_INITIAL (decl) = make_node (BLOCK);
+  BLOCK_SUPERCONTEXT (DECL_INITIAL (decl)) = decl;
+  DECL_STATIC_CONSTRUCTOR (decl) = 1;
+  DECL_ATTRIBUTES (decl) = tree_cons (get_identifier ("constructor"),
+				      NULL_TREE, NULL_TREE);
+
+  tree t = build_decl (UNKNOWN_LOCATION, RESULT_DECL, NULL_TREE,
+		       void_type_node);
+  DECL_ARTIFICIAL (t) = 1;
+  DECL_IGNORED_P (t) = 1;
+  DECL_CONTEXT (t) = decl;
+  DECL_RESULT (decl) = t;
+
+  push_struct_function (decl);
+  init_tree_ssa (cfun);
+
+  tree calldecl = builtin_decl_explicit (BUILT_IN_GOMP_ENABLE_PINNED_MODE);
+  gcall *call = gimple_build_call (calldecl, 0);
+
+  gimple_seq seq = NULL;
+  gimple_seq_add_stmt (&seq, call);
+  gimple_set_body (decl, gimple_build_bind (NULL_TREE, seq, NULL));
+
+  cfun->function_end_locus = UNKNOWN_LOCATION;
+  cfun->curr_properties |= PROP_gimple_any;
+  pop_cfun ();
+  cgraph_node::add_new_function (decl, true);
+}
+
 /* Main entry point.  */
 
 static unsigned int
@@ -14739,6 +14801,10 @@ execute_lower_omp (void)
   for (auto task_stmt : task_cpyfns)
     finalize_task_copyfn (task_stmt);
   task_cpyfns.release ();
+
+  if (flag_offload_memory == OFFLOAD_MEMORY_PINNED)
+    omp_enable_pinned_mode ();
+
   return 0;
 }
 
diff --git a/libgomp/config/linux/allocator.c b/libgomp/config/linux/allocator.c
index edcde9f1e81..8205d67c7a2 100644
--- a/libgomp/config/linux/allocator.c
+++ b/libgomp/config/linux/allocator.c
@@ -51,11 +51,28 @@
 #include <string.h>
 #include "libgomp.h"
 
+static bool always_pinned_mode = false;
+
+/* This function is called by the compiler when -foffload-memory=pinned
+   is used.  */
+
+void
+GOMP_enable_pinned_mode ()
+{
+  if (mlockall (MCL_CURRENT | MCL_FUTURE) != 0)
+    gomp_error ("failed to pin all memory (ulimit too low?)");
+  else
+    always_pinned_mode = true;
+}
+
 static void *
 linux_memspace_alloc (omp_memspace_handle_t memspace, size_t size, int pin)
 {
   (void)memspace;
 
+  /* Explicit pinning may not be required.  */
+  pin = pin && !always_pinned_mode;
+
   if (pin)
     {
       void *addr = mmap (NULL, size, PROT_READ | PROT_WRITE,
@@ -79,6 +96,9 @@ linux_memspace_alloc (omp_memspace_handle_t memspace, size_t size, int pin)
 static void *
 linux_memspace_calloc (omp_memspace_handle_t memspace, size_t size, int pin)
 {
+  /* Explicit pinning may not be required.  */
+  pin = pin && !always_pinned_mode;
+
   if (pin)
     return linux_memspace_alloc (memspace, size, pin);
   else
@@ -91,6 +111,9 @@ linux_memspace_free (omp_memspace_handle_t memspace, void *addr, size_t size,
 {
   (void)memspace;
 
+  /* Explicit pinning may not be required.  */
+  pin = pin && !always_pinned_mode;
+
   if (pin)
     munmap (addr, size);
   else
@@ -101,6 +124,9 @@ static void *
 linux_memspace_realloc (omp_memspace_handle_t memspace, void *addr,
 			size_t oldsize, size_t size, int oldpin, int pin)
 {
+  /* Explicit pinning may not be required.  */
+  pin = pin && !always_pinned_mode;
+
   if (oldpin && pin)
     {
       void *newaddr = mremap (addr, oldsize, size, MREMAP_MAYMOVE);
diff --git a/libgomp/libgomp.map b/libgomp/libgomp.map
index ce6b719a57f..d27d3a30f47 100644
--- a/libgomp/libgomp.map
+++ b/libgomp/libgomp.map
@@ -406,6 +406,7 @@ GOMP_5.0.1 {
   global:
 	GOMP_alloc;
 	GOMP_free;
+	GOMP_enable_pinned_mode;
 } GOMP_5.0;
 
 GOMP_5.1 {
diff --git a/libgomp/testsuite/libgomp.c-c++-common/alloc-pinned-1.c b/libgomp/testsuite/libgomp.c-c++-common/alloc-pinned-1.c
new file mode 100644
index 00000000000..e0e08019bff
--- /dev/null
+++ b/libgomp/testsuite/libgomp.c-c++-common/alloc-pinned-1.c
@@ -0,0 +1,28 @@
+/* { dg-do run } */
+/* { dg-additional-options "-foffload-memory=pinned" } */
+/* { dg-xfail-run-if "Pinning not implemented on this host" { ! *-*-linux-gnu } } */
+
+#if __cplusplus
+#define EXTERNC extern "C"
+#else
+#define EXTERNC
+#endif
+
+/* Intercept the libgomp initialization call to check it happens.  */
+
+int good = 0;
+
+EXTERNC void
+GOMP_enable_pinned_mode ()
+{
+  good = 1;
+}
+
+int
+main ()
+{
+  if (!good)
+    __builtin_exit (1);
+
+  return 0;
+}
diff --git a/libgomp/testsuite/libgomp.c/alloc-pinned-7.c b/libgomp/testsuite/libgomp.c/alloc-pinned-7.c
new file mode 100644
index 00000000000..350bcd36c5a
--- /dev/null
+++ b/libgomp/testsuite/libgomp.c/alloc-pinned-7.c
@@ -0,0 +1,63 @@
+/* { dg-do run } */
+/* { dg-additional-options "-foffload-memory=pinned" } */
+
+/* { dg-xfail-run-if "Pinning not implemented on this host" { ! *-*-linux-gnu } } */
+
+/* Test that -foffload-memory=pinned works.  */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#ifdef __linux__
+#include <sys/types.h>
+#include <unistd.h>
+
+#include <sys/mman.h>
+
+int
+get_pinned_mem ()
+{
+  int pid = getpid ();
+  char buf[100];
+  sprintf (buf, "/proc/%d/status", pid);
+
+  FILE *proc = fopen (buf, "r");
+  if (!proc)
+    abort ();
+  while (fgets (buf, 100, proc))
+    {
+      int val;
+      if (sscanf (buf, "VmLck: %d", &val))
+	{
+	  fclose (proc);
+	  return val;
+	}
+    }
+  abort ();
+}
+#else
+int
+get_pinned_mem ()
+{
+  return 0;
+}
+
+#define mlockall(...) 0
+#endif
+
+#include <omp.h>
+
+int
+main ()
+{
+  // Sanity check
+  if (get_pinned_mem () == 0)
+    {
+      /* -foffload-memory=pinned has failed, but maybe that's because
+	 isufficient pinned memory was available.  */
+      if (mlockall (MCL_CURRENT | MCL_FUTURE) == 0)
+	abort ();
+    }
+
+  return 0;
+}

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 5/6] libgomp, nvptx: Cuda pinned memory
  2023-08-23 14:14 [PATCH v2 0/6] libgomp: OpenMP pinned memory omp_alloc Andrew Stubbs
                   ` (3 preceding siblings ...)
  2023-08-23 14:14 ` [PATCH v2 4/6] openmp: -foffload-memory=pinned Andrew Stubbs
@ 2023-08-23 14:14 ` Andrew Stubbs
  2023-11-22 17:07   ` Tobias Burnus
  2023-08-23 14:14 ` [PATCH v2 6/6] libgomp: fine-grained pinned memory allocator Andrew Stubbs
  5 siblings, 1 reply; 15+ messages in thread
From: Andrew Stubbs @ 2023-08-23 14:14 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 2730 bytes --]


Use Cuda to pin memory, instead of Linux mlock, when available.

There are two advantages: firstly, this gives a significant speed boost for
NVPTX offloading, and secondly, it side-steps the usual OS ulimit/rlimit
setting.

The design adds a device independent plugin API for allocating pinned memory,
and then implements it for NVPTX.  At present, the other supported devices do
not have equivalent capabilities (or requirements).

libgomp/ChangeLog:

	* config/linux/allocator.c: Include assert.h.
	(using_device_for_page_locked): New variable.
	(linux_memspace_alloc): Add init0 parameter. Support device pinning.
	(linux_memspace_calloc): Set init0 to true.
	(linux_memspace_free): Support device pinning.
	(linux_memspace_realloc): Support device pinning.
	(MEMSPACE_ALLOC): Set init0 to false.
	* libgomp-plugin.h
	(GOMP_OFFLOAD_page_locked_host_alloc): New prototype.
	(GOMP_OFFLOAD_page_locked_host_free): Likewise.
	* libgomp.h (gomp_page_locked_host_alloc): Likewise.
	(gomp_page_locked_host_free): Likewise.
	(struct gomp_device_descr): Add page_locked_host_alloc_func and
	page_locked_host_free_func.
	* libgomp_g.h (GOMP_enable_pinned_mode): New prototype.
	* plugin/plugin-nvptx.c
	(GOMP_OFFLOAD_page_locked_host_alloc): New function.
	(GOMP_OFFLOAD_page_locked_host_free): Likewise.
	* target.c (device_for_page_locked): New variable.
	(get_device_for_page_locked): New function.
	(gomp_page_locked_host_alloc): Likewise.
	(gomp_page_locked_host_free): Likewise.
	(gomp_load_plugin_for_device): Add page_locked_host_alloc and
	page_locked_host_free.
	* testsuite/libgomp.c/alloc-pinned-1.c: Change expectations for NVPTX
	devices.
	* testsuite/libgomp.c/alloc-pinned-2.c: Likewise.
	* testsuite/libgomp.c/alloc-pinned-3.c: Likewise.
	* testsuite/libgomp.c/alloc-pinned-4.c: Likewise.
	* testsuite/libgomp.c/alloc-pinned-5.c: Likewise.
	* testsuite/libgomp.c/alloc-pinned-6.c: Likewise.

Co-Authored-By: Thomas Schwinge <thomas@codesourcery.com>
---
 libgomp/config/linux/allocator.c             | 134 ++++++++++++++----
 libgomp/libgomp-plugin.h                     |   2 +
 libgomp/libgomp.h                            |   4 +
 libgomp/libgomp_g.h                          |   1 +
 libgomp/plugin/plugin-nvptx.c                |  34 +++++
 libgomp/target.c                             | 136 +++++++++++++++++++
 libgomp/testsuite/libgomp.c/alloc-pinned-1.c |  25 ++++
 libgomp/testsuite/libgomp.c/alloc-pinned-2.c |  25 ++++
 libgomp/testsuite/libgomp.c/alloc-pinned-3.c |  43 +++++-
 libgomp/testsuite/libgomp.c/alloc-pinned-4.c |  43 +++++-
 libgomp/testsuite/libgomp.c/alloc-pinned-5.c |  25 ++++
 libgomp/testsuite/libgomp.c/alloc-pinned-6.c |  34 ++++-
 12 files changed, 464 insertions(+), 42 deletions(-)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v2-0005-libgomp-nvptx-Cuda-pinned-memory.patch --]
[-- Type: text/x-patch; name="v2-0005-libgomp-nvptx-Cuda-pinned-memory.patch", Size: 27326 bytes --]

diff --git a/libgomp/config/linux/allocator.c b/libgomp/config/linux/allocator.c
index 8205d67c7a2..f29ed1091a3 100644
--- a/libgomp/config/linux/allocator.c
+++ b/libgomp/config/linux/allocator.c
@@ -36,6 +36,11 @@
 
 /* Implement malloc routines that can handle pinned memory on Linux.
    
+   Given that pinned memory is typically used to help host <-> device memory
+   transfers, we attempt to allocate such memory using a device (really:
+   libgomp plugin), but fall back to mmap plus mlock if no suitable device is
+   available.
+
    It's possible to use mlock on any heap memory, but using munlock is
    problematic if there are multiple pinned allocations on the same page.
    Tracking all that manually would be possible, but adds overhead. This may
@@ -49,6 +54,7 @@
 #define _GNU_SOURCE
 #include <sys/mman.h>
 #include <string.h>
+#include <assert.h>
 #include "libgomp.h"
 
 static bool always_pinned_mode = false;
@@ -65,42 +71,87 @@ GOMP_enable_pinned_mode ()
     always_pinned_mode = true;
 }
 
+static int using_device_for_page_locked
+  = /* uninitialized */ -1;
+
 static void *
-linux_memspace_alloc (omp_memspace_handle_t memspace, size_t size, int pin)
+linux_memspace_alloc (omp_memspace_handle_t memspace, size_t size, int pin,
+		      bool init0)
 {
-  (void)memspace;
+  gomp_debug (0, "%s: memspace=%llu, size=%llu, pin=%d, init0=%d\n",
+	      __FUNCTION__, (unsigned long long) memspace,
+	      (unsigned long long) size, pin, init0);
+
+  void *addr;
 
   /* Explicit pinning may not be required.  */
   pin = pin && !always_pinned_mode;
 
   if (pin)
     {
-      void *addr = mmap (NULL, size, PROT_READ | PROT_WRITE,
-			 MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
-      if (addr == MAP_FAILED)
-	return NULL;
-
-      if (mlock (addr, size))
+      int using_device
+	= __atomic_load_n (&using_device_for_page_locked,
+			   MEMMODEL_RELAXED);
+      gomp_debug (0, "  using_device=%d\n",
+		  using_device);
+      if (using_device != 0)
 	{
-	  gomp_debug (0, "libgomp: failed to pin memory (ulimit too low?)\n");
-	  munmap (addr, size);
-	  return NULL;
+	  using_device = gomp_page_locked_host_alloc (&addr, size);
+	  int using_device_old
+	    = __atomic_exchange_n (&using_device_for_page_locked,
+				   using_device, MEMMODEL_RELAXED);
+	  gomp_debug (0, "  using_device=%d, using_device_old=%d\n",
+		      using_device, using_device_old);
+	  assert (using_device_old == -1
+		  /* We shouldn't have concurrently changed our mind.  */
+		  || using_device_old == using_device);
+	}
+      if (using_device == 0)
+	{
+	  gomp_debug (0, "  mmap\n");
+	  addr = mmap (NULL, size, PROT_READ | PROT_WRITE,
+		       MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+	  if (addr == MAP_FAILED)
+	    addr = NULL;
+	  else
+	    {
+	      /* 'mmap' zero-initializes.  */
+	      init0 = false;
+
+	      gomp_debug (0, "  mlock\n");
+	      if (mlock (addr, size))
+		{
+		  gomp_debug (0, "libgomp: failed to pin memory"
+			      " (ulimit too low?)\n");
+		  munmap (addr, size);
+		  addr = NULL;
+		}
+	    }
 	}
-
-      return addr;
     }
   else
-    return malloc (size);
+    addr = malloc (size);
+
+  if (addr && init0)
+    {
+      gomp_debug (0, "  init0\n");
+      memset (addr, 0, size);
+    }
+
+  return addr;
 }
 
 static void *
 linux_memspace_calloc (omp_memspace_handle_t memspace, size_t size, int pin)
 {
+  gomp_debug (0, "%s: memspace=%llu, size=%llu, pin=%d\n",
+	      __FUNCTION__, (unsigned long long) memspace, (unsigned long long) size, pin);
+
   /* Explicit pinning may not be required.  */
   pin = pin && !always_pinned_mode;
 
   if (pin)
-    return linux_memspace_alloc (memspace, size, pin);
+    return linux_memspace_alloc (memspace, size, pin, true);
   else
     return calloc (1, size);
 }
@@ -109,13 +160,25 @@ static void
 linux_memspace_free (omp_memspace_handle_t memspace, void *addr, size_t size,
 		     int pin)
 {
-  (void)memspace;
+  gomp_debug (0, "%s: memspace=%llu, addr=%p, size=%llu, pin=%d\n",
+	      __FUNCTION__, (unsigned long long) memspace, addr, (unsigned long long) size, pin);
 
   /* Explicit pinning may not be required.  */
   pin = pin && !always_pinned_mode;
 
   if (pin)
-    munmap (addr, size);
+    {
+      int using_device
+	= __atomic_load_n (&using_device_for_page_locked,
+			   MEMMODEL_RELAXED);
+      gomp_debug (0, "  using_device=%d\n",
+		  using_device);
+      if (using_device == 1)
+	gomp_page_locked_host_free (addr);
+      else
+	/* 'munlock'ing is implicit with following 'munmap'.  */
+	munmap (addr, size);
+    }
   else
     free (addr);
 }
@@ -124,11 +187,25 @@ static void *
 linux_memspace_realloc (omp_memspace_handle_t memspace, void *addr,
 			size_t oldsize, size_t size, int oldpin, int pin)
 {
+  gomp_debug (0, "%s: memspace=%llu, addr=%p, oldsize=%llu, size=%llu, oldpin=%d, pin=%d\n",
+	      __FUNCTION__, (unsigned long long) memspace, addr, (unsigned long long) oldsize, (unsigned long long) size, oldpin, pin);
+
   /* Explicit pinning may not be required.  */
   pin = pin && !always_pinned_mode;
 
   if (oldpin && pin)
     {
+      /* We can only expect to be able to just 'mremap' if not using a device
+	 for page-locked memory.  */
+      int using_device
+	= __atomic_load_n (&using_device_for_page_locked,
+		       MEMMODEL_RELAXED);
+      gomp_debug (0, "  using_device=%d\n",
+		  using_device);
+      if (using_device != 0)
+	goto manual_realloc;
+
+      gomp_debug (0, "  mremap\n");
       void *newaddr = mremap (addr, oldsize, size, MREMAP_MAYMOVE);
       if (newaddr == MAP_FAILED)
 	return NULL;
@@ -136,22 +213,23 @@ linux_memspace_realloc (omp_memspace_handle_t memspace, void *addr,
       return newaddr;
     }
   else if (oldpin || pin)
-    {
-      void *newaddr = linux_memspace_alloc (memspace, size, pin);
-      if (newaddr)
-	{
-	  memcpy (newaddr, addr, oldsize < size ? oldsize : size);
-	  linux_memspace_free (memspace, addr, oldsize, oldpin);
-	}
-
-      return newaddr;
-    }
+    goto manual_realloc;
   else
     return realloc (addr, size);
+
+manual_realloc:;
+  void *newaddr = linux_memspace_alloc (memspace, size, pin, false);
+  if (newaddr)
+    {
+      memcpy (newaddr, addr, oldsize < size ? oldsize : size);
+      linux_memspace_free (memspace, addr, oldsize, oldpin);
+    }
+
+  return newaddr;
 }
 
 #define MEMSPACE_ALLOC(MEMSPACE, SIZE, PIN) \
-  linux_memspace_alloc (MEMSPACE, SIZE, PIN)
+  linux_memspace_alloc (MEMSPACE, SIZE, PIN, false)
 #define MEMSPACE_CALLOC(MEMSPACE, SIZE, PIN) \
   linux_memspace_calloc (MEMSPACE, SIZE, PIN)
 #define MEMSPACE_REALLOC(MEMSPACE, ADDR, OLDSIZE, SIZE, OLDPIN, PIN) \
diff --git a/libgomp/libgomp-plugin.h b/libgomp/libgomp-plugin.h
index dc993882c3b..b2df4f6d1fd 100644
--- a/libgomp/libgomp-plugin.h
+++ b/libgomp/libgomp-plugin.h
@@ -136,6 +136,8 @@ extern int GOMP_OFFLOAD_load_image (int, unsigned, const void *,
 extern bool GOMP_OFFLOAD_unload_image (int, unsigned, const void *);
 extern void *GOMP_OFFLOAD_alloc (int, size_t);
 extern bool GOMP_OFFLOAD_free (int, void *);
+extern bool GOMP_OFFLOAD_page_locked_host_alloc (void **, size_t);
+extern bool GOMP_OFFLOAD_page_locked_host_free (void *);
 extern bool GOMP_OFFLOAD_dev2host (int, void *, const void *, size_t);
 extern bool GOMP_OFFLOAD_host2dev (int, void *, const void *, size_t);
 extern bool GOMP_OFFLOAD_dev2dev (int, void *, const void *, size_t);
diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
index 61f3ef41be9..3e8015c1a4e 100644
--- a/libgomp/libgomp.h
+++ b/libgomp/libgomp.h
@@ -1128,6 +1128,8 @@ extern int gomp_get_num_devices (void);
 extern bool gomp_target_task_fn (void *);
 extern void gomp_target_rev (uint64_t, uint64_t, uint64_t, uint64_t, uint64_t,
 			     int, struct goacc_asyncqueue *);
+extern bool gomp_page_locked_host_alloc (void **, size_t);
+extern void gomp_page_locked_host_free (void *);
 
 /* Splay tree definitions.  */
 typedef struct splay_tree_node_s *splay_tree_node;
@@ -1383,6 +1385,8 @@ struct gomp_device_descr
   __typeof (GOMP_OFFLOAD_unload_image) *unload_image_func;
   __typeof (GOMP_OFFLOAD_alloc) *alloc_func;
   __typeof (GOMP_OFFLOAD_free) *free_func;
+  __typeof (GOMP_OFFLOAD_page_locked_host_alloc) *page_locked_host_alloc_func;
+  __typeof (GOMP_OFFLOAD_page_locked_host_free) *page_locked_host_free_func;
   __typeof (GOMP_OFFLOAD_dev2host) *dev2host_func;
   __typeof (GOMP_OFFLOAD_host2dev) *host2dev_func;
   __typeof (GOMP_OFFLOAD_memcpy2d) *memcpy2d_func;
diff --git a/libgomp/libgomp_g.h b/libgomp/libgomp_g.h
index 5c1675c7869..c566cecb9cc 100644
--- a/libgomp/libgomp_g.h
+++ b/libgomp/libgomp_g.h
@@ -367,6 +367,7 @@ extern void GOMP_teams_reg (void (*) (void *), void *, unsigned, unsigned,
 
 extern void *GOMP_alloc (size_t, size_t, uintptr_t);
 extern void GOMP_free (void *, uintptr_t);
+extern void GOMP_enable_pinned_mode (void);
 
 /* error.c */
 
diff --git a/libgomp/plugin/plugin-nvptx.c b/libgomp/plugin/plugin-nvptx.c
index 65bd430c5a6..1c7580cca10 100644
--- a/libgomp/plugin/plugin-nvptx.c
+++ b/libgomp/plugin/plugin-nvptx.c
@@ -1549,6 +1549,40 @@ GOMP_OFFLOAD_free (int ord, void *ptr)
 	  && nvptx_free (ptr, ptx_devices[ord]));
 }
 
+bool
+GOMP_OFFLOAD_page_locked_host_alloc (void **ptr, size_t size)
+{
+  GOMP_PLUGIN_debug (0, "nvptx %s: ptr=%p, size=%llu\n",
+		     __FUNCTION__, ptr, (unsigned long long) size);
+
+  CUresult r;
+
+  unsigned int flags = 0;
+  /* Given 'CU_DEVICE_ATTRIBUTE_UNIFIED_ADDRESSING', we don't need
+     'flags |= CU_MEMHOSTALLOC_PORTABLE;' here.  */
+  r = CUDA_CALL_NOCHECK (cuMemHostAlloc, ptr, size, flags);
+  if (r == CUDA_ERROR_OUT_OF_MEMORY)
+    *ptr = NULL;
+  else if (r != CUDA_SUCCESS)
+    {
+      GOMP_PLUGIN_error ("cuMemHostAlloc error: %s", cuda_error (r));
+      return false;
+    }
+  GOMP_PLUGIN_debug (0, "  -> *ptr=%p\n",
+		     *ptr);
+  return true;
+}
+
+bool
+GOMP_OFFLOAD_page_locked_host_free (void *ptr)
+{
+  GOMP_PLUGIN_debug (0, "nvptx %s: ptr=%p\n",
+		     __FUNCTION__, ptr);
+
+  CUDA_CALL (cuMemFreeHost, ptr);
+  return true;
+}
+
 void
 GOMP_OFFLOAD_openacc_exec (void (*fn) (void *),
 			   size_t mapnum  __attribute__((unused)),
diff --git a/libgomp/target.c b/libgomp/target.c
index cd4cc1b01ca..6637161d684 100644
--- a/libgomp/target.c
+++ b/libgomp/target.c
@@ -4334,6 +4334,140 @@ omp_target_free (void *device_ptr, int device_num)
   gomp_mutex_unlock (&devicep->lock);
 }
 
+/* Device (really: libgomp plugin) to use for paged-locked memory.  We
+   assume there is either none or exactly one such device for the lifetime of
+   the process.  */
+
+static struct gomp_device_descr *device_for_page_locked
+  = /* uninitialized */ (void *) -1;
+
+static struct gomp_device_descr *
+get_device_for_page_locked (void)
+{
+  gomp_debug (0, "%s\n",
+	      __FUNCTION__);
+
+  struct gomp_device_descr *device;
+#ifdef HAVE_SYNC_BUILTINS
+  device
+    = __atomic_load_n (&device_for_page_locked, MEMMODEL_RELAXED);
+  if (device == (void *) -1)
+    {
+      gomp_debug (0, "  init\n");
+
+      gomp_init_targets_once ();
+
+      device = NULL;
+      for (int i = 0; i < num_devices; ++i)
+	{
+	  gomp_debug (0, "  i=%d, target_id=%d\n",
+		      i, devices[i].target_id);
+
+	  /* We consider only the first device of potentially several of the
+	     same type as this functionality is not specific to an individual
+	     offloading device, but instead relates to the host-side
+	     implementation of the respective offloading implementation.  */
+	  if (devices[i].target_id != 0)
+	    continue;
+
+	  if (!devices[i].page_locked_host_alloc_func)
+	    continue;
+
+	  gomp_debug (0, "  found device: %p (%s)\n",
+		      &devices[i], devices[i].name);
+	  if (device)
+	    gomp_fatal ("Unclear how %s and %s libgomp plugins may"
+			" simultaneously provide functionality"
+			" for page-locked memory",
+			device->name, devices[i].name);
+	  else
+	    device = &devices[i];
+	}
+
+      struct gomp_device_descr *device_old
+	= __atomic_exchange_n (&device_for_page_locked, device,
+			       MEMMODEL_RELAXED);
+      gomp_debug (0, "  old device_for_page_locked: %p\n",
+		  device_old);
+      assert (device_old == (void *) -1
+	      /* We shouldn't have concurrently found a different or no
+		 device.  */
+	      || device_old == device);
+    }
+#else /* !HAVE_SYNC_BUILTINS */
+  gomp_debug (0, "  not implemented for '!HAVE_SYNC_BUILTINS'\n");
+  (void) &device_for_page_locked;
+  device = NULL;
+#endif /* HAVE_SYNC_BUILTINS */
+
+  gomp_debug (0, "  -> device=%p (%s)\n",
+	      device, device ? device->name : "[none]");
+  return device;
+}
+
+/* Allocate page-locked host memory.
+   Returns whether we have a device capable of that.  */
+
+attribute_hidden bool
+gomp_page_locked_host_alloc (void **ptr, size_t size)
+{
+  gomp_debug (0, "%s: ptr=%p, size=%llu\n",
+	      __FUNCTION__, ptr, (unsigned long long) size);
+
+  struct gomp_device_descr *device = get_device_for_page_locked ();
+  gomp_debug (0, "  device=%p (%s)\n",
+	      device, device ? device->name : "[none]");
+  if (device)
+    {
+      gomp_mutex_lock (&device->lock);
+      if (device->state == GOMP_DEVICE_UNINITIALIZED)
+	gomp_init_device (device);
+      else if (device->state == GOMP_DEVICE_FINALIZED)
+	{
+	  gomp_mutex_unlock (&device->lock);
+	  gomp_fatal ("Device %s used for for page-locked memory is finalized",
+		      device->name);
+	}
+      gomp_mutex_unlock (&device->lock);
+
+      if (!device->page_locked_host_alloc_func (ptr, size))
+	gomp_fatal ("Failed to allocate page-locked host memory"
+		    " via %s libgomp plugin",
+		    device->name);
+    }
+  return device != NULL;
+}
+
+/* Free page-locked host memory.
+   This must only be called if 'gomp_page_locked_host_alloc' returned
+   'true'.  */
+
+attribute_hidden void
+gomp_page_locked_host_free (void *ptr)
+{
+  gomp_debug (0, "%s: ptr=%p\n",
+	      __FUNCTION__, ptr);
+
+  struct gomp_device_descr *device = get_device_for_page_locked ();
+  gomp_debug (0, "  device=%p (%s)\n",
+	      device, device ? device->name : "[none]");
+  assert (device);
+
+  gomp_mutex_lock (&device->lock);
+  assert (device->state != GOMP_DEVICE_UNINITIALIZED);
+  if (device->state == GOMP_DEVICE_FINALIZED)
+    {
+      gomp_mutex_unlock (&device->lock);
+      return;
+    }
+  gomp_mutex_unlock (&device->lock);
+
+  if (!device->page_locked_host_free_func (ptr))
+    gomp_fatal ("Failed to free page-locked host memory"
+		" via %s libgomp plugin",
+		device->name);
+}
+
 int
 omp_target_is_present (const void *ptr, int device_num)
 {
@@ -5056,6 +5190,8 @@ gomp_load_plugin_for_device (struct gomp_device_descr *device,
   DLSYM (unload_image);
   DLSYM (alloc);
   DLSYM (free);
+  DLSYM_OPT (page_locked_host_alloc, page_locked_host_alloc);
+  DLSYM_OPT (page_locked_host_free, page_locked_host_free);
   DLSYM (dev2host);
   DLSYM (host2dev);
   DLSYM_OPT (memcpy2d, memcpy2d);
diff --git a/libgomp/testsuite/libgomp.c/alloc-pinned-1.c b/libgomp/testsuite/libgomp.c/alloc-pinned-1.c
index 8731090bb54..94d89c8f303 100644
--- a/libgomp/testsuite/libgomp.c/alloc-pinned-1.c
+++ b/libgomp/testsuite/libgomp.c/alloc-pinned-1.c
@@ -2,6 +2,8 @@
 
 /* { dg-xfail-run-if "Pinning not implemented on this host" { ! *-*-linux-gnu } } */
 
+/* { dg-additional-options -DOFFLOAD_DEVICE_NVPTX { target offload_device_nvptx } } */
+
 /* Test that pinned memory works.  */
 
 #include <stdio.h>
@@ -67,10 +69,15 @@ verify0 (char *p, size_t s)
 int
 main ()
 {
+#ifdef OFFLOAD_DEVICE_NVPTX
+  /* Go big or go home.  */
+  const int SIZE = 40 * 1024 * 1024;
+#else
   /* Allocate at least a page each time, allowing space for overhead,
      but stay within the ulimit.  */
   const int SIZE = PAGE_SIZE - 128;
   CHECK_SIZE (SIZE * 5);
+#endif
 
   const omp_alloctrait_t traits[] = {
       { omp_atk_pinned, 1 }
@@ -87,21 +94,39 @@ main ()
     abort ();
 
   int amount = get_pinned_mem ();
+#ifdef OFFLOAD_DEVICE_NVPTX
+  /* This doesn't show up as process 'VmLck'ed memory.  */
+  if (amount != 0)
+    abort ();
+#else
   if (amount == 0)
     abort ();
+#endif
 
   p = omp_realloc (p, SIZE * 2, allocator, allocator);
 
   int amount2 = get_pinned_mem ();
+#ifdef OFFLOAD_DEVICE_NVPTX
+  /* This doesn't show up as process 'VmLck'ed memory.  */
+  if (amount2 != 0)
+    abort ();
+#else
   if (amount2 <= amount)
     abort ();
+#endif
 
   /* SIZE*2 ensures that it doesn't slot into the space possibly
      vacated by realloc.  */
   p = omp_calloc (1, SIZE * 2, allocator);
 
+#ifdef OFFLOAD_DEVICE_NVPTX
+  /* This doesn't show up as process 'VmLck'ed memory.  */
+  if (get_pinned_mem () != 0)
+    abort ();
+#else
   if (get_pinned_mem () <= amount2)
     abort ();
+#endif
 
   verify0 (p, SIZE * 2);
 
diff --git a/libgomp/testsuite/libgomp.c/alloc-pinned-2.c b/libgomp/testsuite/libgomp.c/alloc-pinned-2.c
index 27d2403960d..4cd4ccfdbaf 100644
--- a/libgomp/testsuite/libgomp.c/alloc-pinned-2.c
+++ b/libgomp/testsuite/libgomp.c/alloc-pinned-2.c
@@ -2,6 +2,8 @@
 
 /* { dg-xfail-run-if "Pinning not implemented on this host" { ! *-*-linux-gnu } } */
 
+/* { dg-additional-options -DOFFLOAD_DEVICE_NVPTX { target offload_device_nvptx } } */
+
 /* Test that pinned memory works (pool_size code path).  */
 
 #include <stdio.h>
@@ -67,10 +69,15 @@ verify0 (char *p, size_t s)
 int
 main ()
 {
+#ifdef OFFLOAD_DEVICE_NVPTX
+  /* Go big or go home.  */
+  const int SIZE = 40 * 1024 * 1024;
+#else
   /* Allocate at least a page each time, allowing space for overhead,
      but stay within the ulimit.  */
   const int SIZE = PAGE_SIZE - 128;
   CHECK_SIZE (SIZE * 5);
+#endif
 
   const omp_alloctrait_t traits[] = {
       { omp_atk_pinned, 1 },
@@ -88,16 +95,28 @@ main ()
     abort ();
 
   int amount = get_pinned_mem ();
+#ifdef OFFLOAD_DEVICE_NVPTX
+  /* This doesn't show up as process 'VmLck'ed memory.  */
+  if (amount != 0)
+    abort ();
+#else
   if (amount == 0)
     abort ();
+#endif
 
   p = omp_realloc (p, SIZE * 2, allocator, allocator);
   if (!p)
     abort ();
 
   int amount2 = get_pinned_mem ();
+#ifdef OFFLOAD_DEVICE_NVPTX
+  /* This doesn't show up as process 'VmLck'ed memory.  */
+  if (amount2 != 0)
+    abort ();
+#else
   if (amount2 <= amount)
     abort ();
+#endif
 
   /* SIZE*2 ensures that it doesn't slot into the space possibly
      vacated by realloc.  */
@@ -105,8 +124,14 @@ main ()
   if (!p)
     abort ();
 
+#ifdef OFFLOAD_DEVICE_NVPTX
+  /* This doesn't show up as process 'VmLck'ed memory.  */
+  if (get_pinned_mem () != 0)
+    abort ();
+#else
   if (get_pinned_mem () <= amount2)
     abort ();
+#endif
 
   verify0 (p, SIZE * 2);
 
diff --git a/libgomp/testsuite/libgomp.c/alloc-pinned-3.c b/libgomp/testsuite/libgomp.c/alloc-pinned-3.c
index 6521f135c46..138e40c92f0 100644
--- a/libgomp/testsuite/libgomp.c/alloc-pinned-3.c
+++ b/libgomp/testsuite/libgomp.c/alloc-pinned-3.c
@@ -1,5 +1,7 @@
 /* { dg-do run } */
 
+/* { dg-additional-options -DOFFLOAD_DEVICE_NVPTX { target offload_device_nvptx } } */
+
 /* Test that pinned memory fails correctly.  */
 
 #include <stdio.h>
@@ -74,8 +76,14 @@ verify0 (char *p, size_t s)
 int
 main ()
 {
+#ifdef OFFLOAD_DEVICE_NVPTX
+  /* Go big or go home.  */
+  const int SIZE = 40 * 1024 * 1024;
+#else
   /* This needs to be large enough to cover multiple pages.  */
   const int SIZE = PAGE_SIZE * 4;
+#endif
+  const int PIN_LIMIT = PAGE_SIZE * 2;
 
   /* Pinned memory, no fallback.  */
   const omp_alloctrait_t traits1[] = {
@@ -94,21 +102,33 @@ main ()
 							  2, traits2);
 
   /* Ensure that the limit is smaller than the allocation.  */
-  set_pin_limit (SIZE / 2);
+  set_pin_limit (PIN_LIMIT);
 
   // Sanity check
   if (get_pinned_mem () != 0)
     abort ();
 
-  // Should fail
   void *p = omp_alloc (SIZE, allocator1);
+#ifdef OFFLOAD_DEVICE_NVPTX
+  // Doesn't care about 'set_pin_limit'.
+  if (!p)
+    abort ();
+#else
+  // Should fail
   if (p)
     abort ();
+#endif
 
-  // Should fail
   p = omp_calloc (1, SIZE, allocator1);
+#ifdef OFFLOAD_DEVICE_NVPTX
+  // Doesn't care about 'set_pin_limit'.
+  if (!p)
+    abort ();
+#else
+  // Should fail
   if (p)
     abort ();
+#endif
 
   // Should fall back
   p = omp_alloc (SIZE, allocator2);
@@ -121,16 +141,29 @@ main ()
     abort ();
   verify0 (p, SIZE);
 
-  // Should fail to realloc
   void *notpinned = omp_alloc (SIZE, omp_default_mem_alloc);
   p = omp_realloc (notpinned, SIZE, allocator1, omp_default_mem_alloc);
+#ifdef OFFLOAD_DEVICE_NVPTX
+  // Doesn't care about 'set_pin_limit'; does reallocate.
+  if (!notpinned || !p || p == notpinned)
+    abort ();
+#else
+  // Should fail to realloc
   if (!notpinned || p)
     abort ();
+#endif
 
-  // Should fall back to no realloc needed
+#ifdef OFFLOAD_DEVICE_NVPTX
+  void *p_ = omp_realloc (p, SIZE, allocator2, allocator1);
+  // Does reallocate.
+  if (p_ == p)
+    abort ();
+#else
   p = omp_realloc (notpinned, SIZE, allocator2, omp_default_mem_alloc);
+  // Should fall back to no realloc needed
   if (p != notpinned)
     abort ();
+#endif
 
   // No memory should have been pinned
   int amount = get_pinned_mem ();
diff --git a/libgomp/testsuite/libgomp.c/alloc-pinned-4.c b/libgomp/testsuite/libgomp.c/alloc-pinned-4.c
index 51917557c91..38ad6f8e17f 100644
--- a/libgomp/testsuite/libgomp.c/alloc-pinned-4.c
+++ b/libgomp/testsuite/libgomp.c/alloc-pinned-4.c
@@ -1,5 +1,7 @@
 /* { dg-do run } */
 
+/* { dg-additional-options -DOFFLOAD_DEVICE_NVPTX { target offload_device_nvptx } } */
+
 /* Test that pinned memory fails correctly, pool_size code path.  */
 
 #include <stdio.h>
@@ -74,8 +76,14 @@ verify0 (char *p, size_t s)
 int
 main ()
 {
+#ifdef OFFLOAD_DEVICE_NVPTX
+  /* Go big or go home.  */
+  const int SIZE = 40 * 1024 * 1024;
+#else
   /* This needs to be large enough to cover multiple pages.  */
   const int SIZE = PAGE_SIZE * 4;
+#endif
+  const int PIN_LIMIT = PAGE_SIZE * 2;
 
   /* Pinned memory, no fallback.  */
   const omp_alloctrait_t traits1[] = {
@@ -96,21 +104,33 @@ main ()
 							  3, traits2);
 
   /* Ensure that the limit is smaller than the allocation.  */
-  set_pin_limit (SIZE / 2);
+  set_pin_limit (PIN_LIMIT);
 
   // Sanity check
   if (get_pinned_mem () != 0)
     abort ();
 
-  // Should fail
   void *p = omp_alloc (SIZE, allocator1);
+#ifdef OFFLOAD_DEVICE_NVPTX
+  // Doesn't care about 'set_pin_limit'.
+  if (!p)
+    abort ();
+#else
+  // Should fail
   if (p)
     abort ();
+#endif
 
-  // Should fail
   p = omp_calloc (1, SIZE, allocator1);
+#ifdef OFFLOAD_DEVICE_NVPTX
+  // Doesn't care about 'set_pin_limit'.
+  if (!p)
+    abort ();
+#else
+  // Should fail
   if (p)
     abort ();
+#endif
 
   // Should fall back
   p = omp_alloc (SIZE, allocator2);
@@ -123,16 +143,29 @@ main ()
     abort ();
   verify0 (p, SIZE);
 
-  // Should fail to realloc
   void *notpinned = omp_alloc (SIZE, omp_default_mem_alloc);
   p = omp_realloc (notpinned, SIZE, allocator1, omp_default_mem_alloc);
+#ifdef OFFLOAD_DEVICE_NVPTX
+  // Doesn't care about 'set_pin_limit'; does reallocate.
+  if (!notpinned || !p || p == notpinned)
+    abort ();
+#else
+  // Should fail to realloc
   if (!notpinned || p)
     abort ();
+#endif
 
-  // Should fall back to no realloc needed
+#ifdef OFFLOAD_DEVICE_NVPTX
+  void *p_ = omp_realloc (p, SIZE, allocator2, allocator1);
+  // Does reallocate.
+  if (p_ == p)
+    abort ();
+#else
   p = omp_realloc (notpinned, SIZE, allocator2, omp_default_mem_alloc);
+  // Should fall back to no realloc needed
   if (p != notpinned)
     abort ();
+#endif
 
   // No memory should have been pinned
   int amount = get_pinned_mem ();
diff --git a/libgomp/testsuite/libgomp.c/alloc-pinned-5.c b/libgomp/testsuite/libgomp.c/alloc-pinned-5.c
index 9c69dbb7cde..592c86507ca 100644
--- a/libgomp/testsuite/libgomp.c/alloc-pinned-5.c
+++ b/libgomp/testsuite/libgomp.c/alloc-pinned-5.c
@@ -2,6 +2,8 @@
 
 /* { dg-xfail-run-if "Pinning not implemented on this host" { ! *-*-linux-gnu } } */
 
+/* { dg-additional-options -DOFFLOAD_DEVICE_NVPTX { target offload_device_nvptx } } */
+
 /* Test that ompx_pinned_mem_alloc works.  */
 
 #include <stdio.h>
@@ -67,10 +69,15 @@ verify0 (char *p, size_t s)
 int
 main ()
 {
+#ifdef OFFLOAD_DEVICE_NVPTX
+  /* Go big or go home.  */
+  const int SIZE = 40 * 1024 * 1024;
+#else
   /* Allocate at least a page each time, allowing space for overhead,
      but stay within the ulimit.  */
   const int SIZE = PAGE_SIZE - 128;
   CHECK_SIZE (SIZE * 5);
+#endif
 
   // Sanity check
   if (get_pinned_mem () != 0)
@@ -81,21 +88,39 @@ main ()
     abort ();
 
   int amount = get_pinned_mem ();
+#ifdef OFFLOAD_DEVICE_NVPTX
+  /* This doesn't show up as process 'VmLck'ed memory.  */
+  if (amount != 0)
+    abort ();
+#else
   if (amount == 0)
     abort ();
+#endif
 
   p = omp_realloc (p, SIZE * 2, ompx_pinned_mem_alloc, ompx_pinned_mem_alloc);
 
   int amount2 = get_pinned_mem ();
+#ifdef OFFLOAD_DEVICE_NVPTX
+  /* This doesn't show up as process 'VmLck'ed memory.  */
+  if (amount2 != 0)
+    abort ();
+#else
   if (amount2 <= amount)
     abort ();
+#endif
 
   /* SIZE*2 ensures that it doesn't slot into the space possibly
      vacated by realloc.  */
   p = omp_calloc (1, SIZE * 2, ompx_pinned_mem_alloc);
 
+#ifdef OFFLOAD_DEVICE_NVPTX
+  /* This doesn't show up as process 'VmLck'ed memory.  */
+  if (get_pinned_mem () != 0)
+    abort ();
+#else
   if (get_pinned_mem () <= amount2)
     abort ();
+#endif
 
   verify0 (p, SIZE * 2);
 
diff --git a/libgomp/testsuite/libgomp.c/alloc-pinned-6.c b/libgomp/testsuite/libgomp.c/alloc-pinned-6.c
index f80a0264f97..6524d04ce2e 100644
--- a/libgomp/testsuite/libgomp.c/alloc-pinned-6.c
+++ b/libgomp/testsuite/libgomp.c/alloc-pinned-6.c
@@ -1,5 +1,7 @@
 /* { dg-do run } */
 
+/* { dg-additional-options -DOFFLOAD_DEVICE_NVPTX { target offload_device_nvptx } } */
+
 /* Test that ompx_pinned_mem_alloc fails correctly.  */
 
 #include <stdio.h>
@@ -66,31 +68,55 @@ set_pin_limit ()
 int
 main ()
 {
+#ifdef OFFLOAD_DEVICE_NVPTX
+  /* Go big or go home.  */
+  const int SIZE = 40 * 1024 * 1024;
+#else
   /* Allocate at least a page each time, but stay within the ulimit.  */
   const int SIZE = PAGE_SIZE * 4;
+#endif
+  const int PIN_LIMIT = PAGE_SIZE*2;
 
   /* Ensure that the limit is smaller than the allocation.  */
-  set_pin_limit (SIZE / 2);
+  set_pin_limit (PIN_LIMIT);
 
   // Sanity check
   if (get_pinned_mem () != 0)
     abort ();
 
-  // Should fail
   void *p = omp_alloc (SIZE, ompx_pinned_mem_alloc);
+#ifdef OFFLOAD_DEVICE_NVPTX
+  // Doesn't care about 'set_pin_limit'.
+  if (!p)
+    abort ();
+#else
+  // Should fail
   if (p)
     abort ();
+#endif
 
-  // Should fail
   p = omp_calloc (1, SIZE, ompx_pinned_mem_alloc);
+#ifdef OFFLOAD_DEVICE_NVPTX
+  // Doesn't care about 'set_pin_limit'.
+  if (!p)
+    abort ();
+#else
+  // Should fail
   if (p)
     abort ();
+#endif
 
-  // Should fail to realloc
   void *notpinned = omp_alloc (SIZE, omp_default_mem_alloc);
   p = omp_realloc (notpinned, SIZE, ompx_pinned_mem_alloc, omp_default_mem_alloc);
+#ifdef OFFLOAD_DEVICE_NVPTX
+  // Doesn't care about 'set_pin_limit'; does reallocate.
+  if (!notpinned || !p || p == notpinned)
+    abort ();
+#else
+  // Should fail to realloc
   if (!notpinned || p)
     abort ();
+#endif
 
   // No memory should have been pinned
   int amount = get_pinned_mem ();

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 6/6] libgomp: fine-grained pinned memory allocator
  2023-08-23 14:14 [PATCH v2 0/6] libgomp: OpenMP pinned memory omp_alloc Andrew Stubbs
                   ` (4 preceding siblings ...)
  2023-08-23 14:14 ` [PATCH v2 5/6] libgomp, nvptx: Cuda pinned memory Andrew Stubbs
@ 2023-08-23 14:14 ` Andrew Stubbs
  5 siblings, 0 replies; 15+ messages in thread
From: Andrew Stubbs @ 2023-08-23 14:14 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 2705 bytes --]


This patch introduces a new custom memory allocator for use with pinned
memory (in the case where the Cuda allocator isn't available).  In future,
this allocator will also be used for Unified Shared Memory.  Both memories
are incompatible with the system malloc because allocated memory cannot
share a page with memory allocated for other purposes.

This means that small allocations will no longer consume an entire page of
pinned memory.  Unfortunately, it also means that pinned memory pages will
never be unmapped (although they may be reused).

The implementation is not perfect; there are various corner cases (especially
related to extending onto new pages) where allocations and reallocations may
be sub-optimal, but it should still be a step forward in support for small
allocations.

I have considered using libmemkind's "fixed" memory but rejected it for three
reasons: 1) libmemkind may not always be present at runtime, 2) there's no
currently documented means to extend a "fixed" kind one page at a time
(although the code appears to have an undocumented function that may do the
job, and/or extending libmemkind to support the MAP_LOCKED mmap flag with its
regular kinds would be straight-forward), 3) USM benefits from having the
metadata located in different memory and using an external implementation makes
it hard to guarantee this.

libgomp/ChangeLog:

	* Makefile.am (libgomp_la_SOURCES): Add usmpin-allocator.c.
	* Makefile.in: Regenerate.
	* config/linux/allocator.c: Include unistd.h.
	(pin_ctx): New variable.
	(ctxlock): New variable.
	(linux_init_pin_ctx): New function.
	(linux_memspace_alloc): Use usmpin-allocator for pinned memory.
	(linux_memspace_free): Likewise.
	(linux_memspace_realloc): Likewise.
	* libgomp.h (usmpin_init_context): New prototype.
	(usmpin_register_memory): New prototype.
	(usmpin_alloc): New prototype.
	(usmpin_free): New prototype.
	(usmpin_realloc): New prototype.
	* testsuite/libgomp.c/alloc-pinned-1.c: Adjust for new behaviour.
	* testsuite/libgomp.c/alloc-pinned-2.c: Likewise.
	* testsuite/libgomp.c/alloc-pinned-5.c: Likewise.
	* testsuite/libgomp.c/alloc-pinned-8.c: New test.
	* usmpin-allocator.c: New file.
---
 libgomp/Makefile.am                          |   2 +-
 libgomp/Makefile.in                          |   5 +-
 libgomp/config/linux/allocator.c             |  91 ++++--
 libgomp/libgomp.h                            |  10 +
 libgomp/testsuite/libgomp.c/alloc-pinned-8.c | 127 ++++++++
 libgomp/usmpin-allocator.c                   | 319 +++++++++++++++++++
 6 files changed, 521 insertions(+), 33 deletions(-)
 create mode 100644 libgomp/testsuite/libgomp.c/alloc-pinned-8.c
 create mode 100644 libgomp/usmpin-allocator.c


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v2-0006-libgomp-fine-grained-pinned-memory-allocator.patch --]
[-- Type: text/x-patch; name="v2-0006-libgomp-fine-grained-pinned-memory-allocator.patch", Size: 19854 bytes --]

diff --git a/libgomp/Makefile.am b/libgomp/Makefile.am
index 428f7a9dab5..2402739e07d 100644
--- a/libgomp/Makefile.am
+++ b/libgomp/Makefile.am
@@ -67,7 +67,7 @@ libgomp_la_SOURCES = alloc.c atomic.c barrier.c critical.c env.c error.c \
 	target.c splay-tree.c libgomp-plugin.c oacc-parallel.c oacc-host.c \
 	oacc-init.c oacc-mem.c oacc-async.c oacc-plugin.c oacc-cuda.c \
 	priority_queue.c affinity-fmt.c teams.c allocator.c oacc-profiling.c \
-	oacc-target.c
+	oacc-target.c usmpin-allocator.c
 
 include $(top_srcdir)/plugin/Makefrag.am
 
diff --git a/libgomp/Makefile.in b/libgomp/Makefile.in
index 3ef05e6a3cb..8b3aa3c8499 100644
--- a/libgomp/Makefile.in
+++ b/libgomp/Makefile.in
@@ -219,7 +219,7 @@ am_libgomp_la_OBJECTS = alloc.lo atomic.lo barrier.lo critical.lo \
 	oacc-parallel.lo oacc-host.lo oacc-init.lo oacc-mem.lo \
 	oacc-async.lo oacc-plugin.lo oacc-cuda.lo priority_queue.lo \
 	affinity-fmt.lo teams.lo allocator.lo oacc-profiling.lo \
-	oacc-target.lo $(am__objects_1)
+	oacc-target.lo usmpin-allocator.lo $(am__objects_1)
 libgomp_la_OBJECTS = $(am_libgomp_la_OBJECTS)
 AM_V_P = $(am__v_P_@AM_V@)
 am__v_P_ = $(am__v_P_@AM_DEFAULT_V@)
@@ -549,7 +549,7 @@ libgomp_la_SOURCES = alloc.c atomic.c barrier.c critical.c env.c \
 	oacc-parallel.c oacc-host.c oacc-init.c oacc-mem.c \
 	oacc-async.c oacc-plugin.c oacc-cuda.c priority_queue.c \
 	affinity-fmt.c teams.c allocator.c oacc-profiling.c \
-	oacc-target.c $(am__append_3)
+	oacc-target.c usmpin-allocator.c $(am__append_3)
 
 # Nvidia PTX OpenACC plugin.
 @PLUGIN_NVPTX_TRUE@libgomp_plugin_nvptx_version_info = -version-info $(libtool_VERSION)
@@ -782,6 +782,7 @@ distclean-compile:
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/team.Plo@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/teams.Plo@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/time.Plo@am__quote@
+@AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/usmpin-allocator.Plo@am__quote@
 @AMDEP_TRUE@@am__include@ @am__quote@./$(DEPDIR)/work.Plo@am__quote@
 
 .c.o:
diff --git a/libgomp/config/linux/allocator.c b/libgomp/config/linux/allocator.c
index f29ed1091a3..df18ad03d68 100644
--- a/libgomp/config/linux/allocator.c
+++ b/libgomp/config/linux/allocator.c
@@ -53,6 +53,7 @@
 
 #define _GNU_SOURCE
 #include <sys/mman.h>
+#include <unistd.h>
 #include <string.h>
 #include <assert.h>
 #include "libgomp.h"
@@ -74,6 +75,16 @@ GOMP_enable_pinned_mode ()
 static int using_device_for_page_locked
   = /* uninitialized */ -1;
 
+
+static usmpin_ctx_p pin_ctx = NULL;
+static pthread_once_t ctxlock = PTHREAD_ONCE_INIT;
+
+static void
+linux_init_pin_ctx ()
+{
+  pin_ctx = usmpin_init_context ();
+}
+
 static void *
 linux_memspace_alloc (omp_memspace_handle_t memspace, size_t size, int pin,
 		      bool init0)
@@ -82,7 +93,7 @@ linux_memspace_alloc (omp_memspace_handle_t memspace, size_t size, int pin,
 	      __FUNCTION__, (unsigned long long) memspace,
 	      (unsigned long long) size, pin, init0);
 
-  void *addr;
+  void *addr = NULL;
 
   /* Explicit pinning may not be required.  */
   pin = pin && !always_pinned_mode;
@@ -108,23 +119,44 @@ linux_memspace_alloc (omp_memspace_handle_t memspace, size_t size, int pin,
 	}
       if (using_device == 0)
 	{
-	  gomp_debug (0, "  mmap\n");
-	  addr = mmap (NULL, size, PROT_READ | PROT_WRITE,
-		       MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
-	  if (addr == MAP_FAILED)
-	    addr = NULL;
-	  else
-	    {
-	      /* 'mmap' zero-initializes.  */
-	      init0 = false;
+	  static int pagesize = 0;
+	  static void *addrhint = NULL;
 
-	      gomp_debug (0, "  mlock\n");
-	      if (mlock (addr, size))
+	  if (!pagesize)
+	    pagesize = sysconf(_SC_PAGE_SIZE);
+ 
+	  while (1)
+	    {
+	      addr = usmpin_alloc (pin_ctx, size);
+	      if (addr)
+		break;
+
+	      gomp_debug (0, "  mmap\n");
+
+	      /* Round up to a whole page.  */
+	      size_t misalignment = size % pagesize;
+	      size_t mmap_size = (misalignment > 0
+				  ? size + pagesize - misalignment
+				  : size);
+	      void *newpage = mmap (addrhint, mmap_size, PROT_READ | PROT_WRITE,
+				 MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+	      if (newpage == MAP_FAILED)
+		break;
+	      else
 		{
-		  gomp_debug (0, "libgomp: failed to pin memory"
-			      " (ulimit too low?)\n");
-		  munmap (addr, size);
-		  addr = NULL;
+		  gomp_debug (0, "  mlock\n");
+		  if (mlock (newpage, size))
+		    {
+		      gomp_debug (0, "libgomp: failed to pin memory"
+				  " (ulimit too low?)\n");
+		      munmap (newpage, size);
+		      break;
+		    }
+
+		  addrhint = newpage + mmap_size;
+
+		  pthread_once (&ctxlock, linux_init_pin_ctx);
+		  usmpin_register_memory (pin_ctx, newpage, mmap_size);
 		}
 	    }
 	}
@@ -176,8 +208,7 @@ linux_memspace_free (omp_memspace_handle_t memspace, void *addr, size_t size,
       if (using_device == 1)
 	gomp_page_locked_host_free (addr);
       else
-	/* 'munlock'ing is implicit with following 'munmap'.  */
-	munmap (addr, size);
+	usmpin_free (pin_ctx, addr);
     }
   else
     free (addr);
@@ -195,29 +226,29 @@ linux_memspace_realloc (omp_memspace_handle_t memspace, void *addr,
 
   if (oldpin && pin)
     {
-      /* We can only expect to be able to just 'mremap' if not using a device
-	 for page-locked memory.  */
       int using_device
 	= __atomic_load_n (&using_device_for_page_locked,
 		       MEMMODEL_RELAXED);
       gomp_debug (0, "  using_device=%d\n",
 		  using_device);
-      if (using_device != 0)
-	goto manual_realloc;
-
-      gomp_debug (0, "  mremap\n");
-      void *newaddr = mremap (addr, oldsize, size, MREMAP_MAYMOVE);
-      if (newaddr == MAP_FAILED)
-	return NULL;
 
-      return newaddr;
+      /* The device plugin API does not support realloc,
+	 but the usmpin allocator does.  */
+      if (using_device == 0)
+	{
+	  /* This can fail if there is insufficient pinned memory free.  */
+	  void *newaddr = usmpin_realloc (pin_ctx, addr, size);
+	  if (newaddr)
+	    return newaddr;
+	}
     }
   else if (oldpin || pin)
-    goto manual_realloc;
+    /* Moving from pinned to unpinned memory cannot be done in-place.  */
+    ;
   else
     return realloc (addr, size);
 
-manual_realloc:;
+  /* In-place reallocation failed.  Fall back to copy.  */
   void *newaddr = linux_memspace_alloc (memspace, size, pin, false);
   if (newaddr)
     {
diff --git a/libgomp/libgomp.h b/libgomp/libgomp.h
index 3e8015c1a4e..98552f838db 100644
--- a/libgomp/libgomp.h
+++ b/libgomp/libgomp.h
@@ -1632,4 +1632,14 @@ gomp_thread_to_pthread_t (struct gomp_thread *thr)
 }
 #endif
 
+/* usmpin-allocator.c  */
+
+typedef struct usmpin_context *usmpin_ctx_p;
+
+usmpin_ctx_p usmpin_init_context ();
+void usmpin_register_memory (usmpin_ctx_p ctx, char *base, size_t size);
+void *usmpin_alloc (usmpin_ctx_p ctx, size_t size);
+void usmpin_free (usmpin_ctx_p ctx, void *addr);
+void *usmpin_realloc (usmpin_ctx_p ctx, void *addr, size_t newsize);
+
 #endif /* LIBGOMP_H */
diff --git a/libgomp/testsuite/libgomp.c/alloc-pinned-8.c b/libgomp/testsuite/libgomp.c/alloc-pinned-8.c
new file mode 100644
index 00000000000..fc5986805c7
--- /dev/null
+++ b/libgomp/testsuite/libgomp.c/alloc-pinned-8.c
@@ -0,0 +1,127 @@
+/* { dg-do run } */
+
+/* { dg-xfail-run-if "Pinning not implemented on this host" { ! *-*-linux-gnu } } */
+
+/* { dg-additional-options -DOFFLOAD_DEVICE_NVPTX { target offload_device_nvptx } } */
+
+/* Test that pinned memory works for small allocations.  */
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#ifdef __linux__
+#include <sys/types.h>
+#include <unistd.h>
+
+#include <sys/mman.h>
+#include <sys/resource.h>
+
+#define PAGE_SIZE sysconf(_SC_PAGESIZE)
+#define CHECK_SIZE(SIZE) { \
+  struct rlimit limit; \
+  if (getrlimit (RLIMIT_MEMLOCK, &limit) \
+      || limit.rlim_cur <= SIZE) \
+    fprintf (stderr, "unsufficient lockable memory; please increase ulimit\n"); \
+  }
+
+int
+get_pinned_mem ()
+{
+  int pid = getpid ();
+  char buf[100];
+  sprintf (buf, "/proc/%d/status", pid);
+
+  FILE *proc = fopen (buf, "r");
+  if (!proc)
+    abort ();
+  while (fgets (buf, 100, proc))
+    {
+      int val;
+      if (sscanf (buf, "VmLck: %d", &val))
+	{
+	  fclose (proc);
+	  return val;
+	}
+    }
+  abort ();
+}
+#else
+#define PAGE_SIZE 1 /* unknown */
+#define CHECK_SIZE(SIZE) fprintf (stderr, "OS unsupported\n");
+
+int
+get_pinned_mem ()
+{
+  return 0;
+}
+#endif
+
+static void
+verify0 (char *p, size_t s)
+{
+  for (size_t i = 0; i < s; ++i)
+    if (p[i] != 0)
+      abort ();
+}
+
+#include <omp.h>
+
+int
+main ()
+{
+  /* Choose a small size where all our allocations fit on one page.  */
+  const int SIZE = 10;
+  CHECK_SIZE (SIZE*4);
+
+  const omp_alloctrait_t traits[] = {
+      { omp_atk_pinned, 1 }
+  };
+  omp_allocator_handle_t allocator = omp_init_allocator (omp_default_mem_space, 1, traits);
+
+  // Sanity check
+  if (get_pinned_mem () != 0)
+    abort ();
+
+  void *p = omp_alloc (SIZE, allocator);
+  if (!p)
+    abort ();
+
+  int amount = get_pinned_mem ();
+#ifdef OFFLOAD_DEVICE_NVPTX
+  /* This doesn't show up as process 'VmLck'ed memory.  */
+  if (amount != 0)
+    abort ();
+#else
+  if (amount == 0)
+    abort ();
+#endif
+
+  p = omp_realloc (p, SIZE * 2, allocator, allocator);
+
+  int amount2 = get_pinned_mem ();
+#ifdef OFFLOAD_DEVICE_NVPTX
+  /* This doesn't show up as process 'VmLck'ed memory.  */
+  if (amount2 != 0)
+    abort ();
+#else
+  /* A small allocation should not allocate another page.  */
+  if (amount2 != amount)
+    abort ();
+#endif
+
+  p = omp_calloc (1, SIZE, allocator);
+
+#ifdef OFFLOAD_DEVICE_NVPTX
+  /* This doesn't show up as process 'VmLck'ed memory.  */
+  if (get_pinned_mem () != 0)
+    abort ();
+#else
+  /* A small allocation should not allocate another page.  */
+  if (get_pinned_mem () != amount2)
+    abort ();
+#endif
+
+  verify0 (p, SIZE);
+
+  return 0;
+}
diff --git a/libgomp/usmpin-allocator.c b/libgomp/usmpin-allocator.c
new file mode 100644
index 00000000000..311bda5054e
--- /dev/null
+++ b/libgomp/usmpin-allocator.c
@@ -0,0 +1,319 @@
+/* Copyright (C) 2023 Free Software Foundation, Inc.
+
+   This file is part of the GNU Offloading and Multi Processing Library
+   (libgomp).
+
+   Libgomp is free software; you can redistribute it and/or modify it
+   under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3, or (at your option)
+   any later version.
+
+   Libgomp is distributed in the hope that it will be useful, but WITHOUT ANY
+   WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
+   FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+   more details.
+
+   Under Section 7 of GPL version 3, you are granted additional
+   permissions described in the GCC Runtime Library Exception, version
+   3.1, as published by the Free Software Foundation.
+
+   You should have received a copy of the GNU General Public License and
+   a copy of the GCC Runtime Library Exception along with this program;
+   see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
+   <http://www.gnu.org/licenses/>.  */
+
+/* This is a simple "malloc" implementation intended for use with Unified
+   Shared Memory and Pinned Memory.  It allocates memory from a pool allocated
+   and configured by the device plugin (for USM), or the OS-specific allocator
+   (for pinned).
+ 
+   This implementation keeps the allocated/free chain in a side-table (splay
+   tree) to ensure that the allocation routine does not migrate all the USM
+   pages back into host memory.  Keeping the meta-data elsewhere is also useful
+   for pinned memory, which is typically an extremely limited resource.  */
+
+#include <string.h>
+#include "libgomp.h"
+
+/* Use a splay tree to track allocations.  */
+
+typedef struct usmpin_splay_tree_node_s *usmpin_splay_tree_node;
+typedef struct usmpin_splay_tree_s *usmpin_splay_tree;
+typedef struct usmpin_splay_tree_key_s *usmpin_splay_tree_key;
+
+struct usmpin_splay_tree_key_s {
+  void *base;
+  size_t size;
+};
+
+static inline int
+usmpin_splay_compare (usmpin_splay_tree_key x, usmpin_splay_tree_key y)
+{
+  return (x->base == y->base ? 0
+	  : x->base > y->base ? 1
+	  : -1);
+}
+
+#define splay_tree_prefix usmpin
+#include "splay-tree.h"
+
+/* 128-byte granularity means GPU cache-line aligned.  */
+#define ALIGN(VAR) (((VAR) + 127) & ~127)
+
+/* The context data prevents the need for global state.  */
+struct usmpin_context {
+  int lock;
+  struct usmpin_splay_tree_s allocations;
+  struct usmpin_splay_tree_s free_space;
+};
+
+usmpin_ctx_p
+usmpin_init_context ()
+{
+  return calloc (1, sizeof (struct usmpin_context));
+}
+
+/* Coalesce contiguous free space into one entry.  This considers the entries
+   either side of the root node only, so it should be called each time a new
+   entry in inserted into the root.  */
+
+static void
+usmpin_coalesce_free_space (usmpin_ctx_p ctx)
+{
+  usmpin_splay_tree_node prev, next, node = ctx->free_space.root;
+
+  for (prev = node->left; prev && prev->right; prev = prev->right)
+    ;
+  for (next = node->right; next && next->left; next = next->left)
+    ;
+
+  /* Coalesce adjacent free chunks.  */
+  if (next
+      && node->key.base + node->key.size == next->key.base)
+    {
+      /* Free chunk follows.  */
+      node->key.size += next->key.size;
+      usmpin_splay_tree_remove (&ctx->free_space, &next->key);
+      free (next);
+    }
+  if (prev
+      && prev->key.base + prev->key.size == node->key.base)
+    {
+      /* Free chunk precedes.  */
+      prev->key.size += node->key.size;
+      usmpin_splay_tree_remove (&ctx->free_space, &node->key);
+      free (node);
+    }
+}
+
+/* Add a new memory region into the free chain.  This is how the USM heap is
+   initialized and extended.  If the new region is contiguous with an existing
+   region then any free space will be coalesced.  */
+
+void
+usmpin_register_memory (usmpin_ctx_p ctx, char *base, size_t size)
+{
+  if (base == NULL || ctx == NULL)
+    return;
+
+  while (__atomic_exchange_n (&ctx->lock, 1, MEMMODEL_ACQUIRE) == 1)
+    ;
+
+  usmpin_splay_tree_node node;
+  node = malloc (sizeof (struct usmpin_splay_tree_node_s));
+  node->key.base = base;
+  node->key.size = size;
+  node->left = NULL;
+  node->right = NULL;
+  usmpin_splay_tree_insert (&ctx->free_space, node);
+  usmpin_coalesce_free_space (ctx);
+
+  __atomic_store_n (&ctx->lock, 0, MEMMODEL_RELEASE);
+}
+
+/* This splay_tree_foreach callback selects the first free space large enough
+   to hold the allocation needed.  Since the splay_tree walk may start in the
+   middle the "first" isn't necessarily the "leftmost" entry.  */
+
+struct usmpin_callback_data {
+  size_t size;
+  usmpin_splay_tree_node found;
+};
+
+static int
+usmpin_alloc_callback (usmpin_splay_tree_key key, void *data)
+{
+  struct usmpin_callback_data *cbd = (struct usmpin_callback_data *)data;
+
+  if (key->size >= cbd->size)
+    {
+      cbd->found = (usmpin_splay_tree_node)key;
+      return 1;
+    }
+
+  return 0;
+}
+
+/* USM "malloc".  Selects and moves and address range from ctx->free_space to
+   ctx->allocations, while leaving any excess in ctx->free_space.  */
+
+void *
+usmpin_alloc (usmpin_ctx_p ctx, size_t size)
+{
+  if (ctx == NULL)
+    return NULL;
+
+  /* Memory is allocated in N-byte granularity.  */
+  size = ALIGN (size);
+
+  /* Acquire the lock.  */
+  while (__atomic_exchange_n (&ctx->lock, 1, MEMMODEL_ACQUIRE) == 1)
+    ;
+
+  if (!ctx->free_space.root)
+    {
+      /* No memory registered, or no free space.  */
+      __atomic_store_n (&ctx->lock, 0, MEMMODEL_RELEASE);
+      return NULL;
+    }
+
+  /* Find a suitable free block.  */
+  struct usmpin_callback_data cbd = {size, NULL};
+  usmpin_splay_tree_foreach_lazy (&ctx->free_space, usmpin_alloc_callback,
+				  &cbd);
+  usmpin_splay_tree_node freenode = cbd.found;
+
+  void *result = NULL;
+  if (freenode)
+    {
+      /* Allocation successful.  */
+      result = freenode->key.base;
+      usmpin_splay_tree_node allocnode = malloc (sizeof (*allocnode));
+      allocnode->key.base = result;
+      allocnode->key.size = size;
+      allocnode->left = NULL;
+      allocnode->right = NULL;
+      usmpin_splay_tree_insert (&ctx->allocations, allocnode);
+
+      /* Update the free chain.  */
+      size_t stillfree_size = freenode->key.size - size;
+      if (stillfree_size > 0)
+	{
+	  freenode->key.base = freenode->key.base + size;
+	  freenode->key.size = stillfree_size;
+	}
+      else
+	{
+	  usmpin_splay_tree_remove (&ctx->free_space, &freenode->key);
+	  free (freenode);
+	}
+    }
+
+  /* Release the lock.  */
+  __atomic_store_n (&ctx->lock, 0, MEMMODEL_RELEASE);
+
+  return result;
+}
+
+/* USM "free".  Moves an address range from ctx->allocations to
+   ctx->free_space and merges that record with any contiguous free memory.  */
+
+void
+usmpin_free (usmpin_ctx_p ctx, void *addr)
+{
+  if (ctx == NULL)
+    return;
+
+  /* Acquire the lock.  */
+  while (__atomic_exchange_n (&ctx->lock, 1, MEMMODEL_ACQUIRE) == 1)
+    ;
+
+  /* Convert the memory map to free.  */
+  struct usmpin_splay_tree_key_s key = {addr};
+  usmpin_splay_tree_key found = usmpin_splay_tree_lookup (&ctx->allocations,
+							  &key);
+  if (!found)
+    GOMP_PLUGIN_fatal ("invalid free");
+  usmpin_splay_tree_remove (&ctx->allocations, &key);
+  usmpin_splay_tree_insert (&ctx->free_space, (usmpin_splay_tree_node)found);
+  usmpin_coalesce_free_space (ctx);
+
+  /* Release the lock.  */
+  __atomic_store_n (&ctx->lock, 0, MEMMODEL_RELEASE);
+}
+
+/* USM "realloc".  Works in-place, if possible; reallocates otherwise.  */
+
+void *
+usmpin_realloc (usmpin_ctx_p ctx, void *addr, size_t newsize)
+{
+  if (ctx == NULL)
+    return NULL;
+
+  newsize = ALIGN (newsize);
+
+  /* Acquire the lock.  */
+  while (__atomic_exchange_n (&ctx->lock, 1, MEMMODEL_ACQUIRE) == 1)
+    ;
+
+  /* Convert the memory map to free.  */
+  struct usmpin_splay_tree_key_s key = {addr};
+  usmpin_splay_tree_key found = usmpin_splay_tree_lookup (&ctx->allocations,
+							  &key);
+  if (!found)
+    GOMP_PLUGIN_fatal ("invalid realloc");
+
+  if (newsize == found->size)
+    ; /* Nothing to do.  */
+  else if (newsize < found->size)
+    {
+      /* We're reducing the allocation size.  */
+      usmpin_splay_tree_node newfree = malloc (sizeof (*newfree));
+      newfree->key.base = found->base + newsize;
+      newfree->key.size = found->size - newsize;
+      newfree->left = NULL;
+      newfree->right = NULL;
+      usmpin_splay_tree_insert (&ctx->free_space, newfree);
+      usmpin_coalesce_free_space (ctx);
+    }
+  else
+    {
+      /* We're extending the allocation.  */
+      struct usmpin_splay_tree_key_s freekey = {addr + found->size};
+      usmpin_splay_tree_key foundfree;
+      foundfree = usmpin_splay_tree_lookup (&ctx->free_space, &freekey);
+      if (foundfree && foundfree->size >= newsize - found->size)
+	{
+	  /* Allocation can be expanded in place.  */
+	  foundfree->base += found->size;
+	  foundfree->size -= newsize - found->size;
+	  found->size = newsize;
+
+	  if (foundfree->size == 0)
+	    usmpin_splay_tree_remove (&ctx->free_space, &freekey);
+	}
+      else
+	{
+	  /* Allocation must be relocated.
+	     Release the lock and use alloc/free.  */
+	  __atomic_store_n (&ctx->lock, 0, MEMMODEL_RELEASE);
+
+	  void *newaddr = usmpin_alloc (ctx, newsize);
+	  if (!newaddr)
+	    return NULL;
+
+	  memcpy (newaddr, addr, found->size);
+	  usmpin_free (ctx, addr);
+	  return newaddr;
+	}
+    }
+
+  /* Release the lock.  */
+  __atomic_store_n (&ctx->lock, 0, MEMMODEL_RELEASE);
+  return addr;
+}
+
+/* Include the splay tree code inline, with the prefixes added.  */
+#define splay_tree_prefix usmpin
+#define splay_tree_c
+#include "splay-tree.h"

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/6] libgomp: basic pinned memory on Linux
  2023-08-23 14:14 ` [PATCH v2 1/6] libgomp: basic pinned memory on Linux Andrew Stubbs
@ 2023-11-22 14:26   ` Tobias Burnus
  2023-11-29 17:05     ` Andrew Stubbs
  0 siblings, 1 reply; 15+ messages in thread
From: Tobias Burnus @ 2023-11-22 14:26 UTC (permalink / raw)
  To: Andrew Stubbs, gcc-patches; +Cc: Jakub Jelinek

Hi Andrew,

Side remark:

> -#define MEMSPACE_CALLOC(MEMSPACE, SIZE) \ - calloc (1,
> (((void)(MEMSPACE), (SIZE))))

This fits a bit more to previous patch, but I wonder whether that should
use (MEMSPACE, NMEMB, SIZE) instead - to fit to the actual calloc arguments.

I think the main/only difference between SIZE and NMEMB and SIZE is that
"If the multiplication of nmemb and size would result in integer overflow,
then calloc() returns an error." (Linux manpage)

However, while this wording seems to be neither in POSIX nor in the OpenMP
spec. There was some alignment discussion at https://gcc.gnu.org/PR112364
regarding whether C (since C23) has a different alignment for
calloc(1, n) vs. calloc(n,1) but Joseph believes it doen't.

Thus, this is more bikesheding than making a real difference.

* * *

[somehow my email program caused some odd formatting issues when I
hit some odd key combo. I am not sure whether I fully fixed it or not;
sorry if some parts look odd.]


On 23.08.23 16:14, Andrew Stubbs wrote:
> Implement the OpenMP pinned memory trait on Linux hosts using the
> mlock syscall.  Pinned allocations are performed using mmap, not
> malloc, to ensure that they can be unpinned safely when freed.
>
> This implementation will work OK for page-scale allocations, and
> finer-grained allocations will be implemented in a future patch.

Can you also update libgomp.texi, i.e. https://gcc.gnu.org/onlinedocs/libgomp/Memory-allocation.html
to document that and how pinning works on Linux?

I think I proposed in the low-latency patch to add a @ref to
https://gcc.gnu.org/onlinedocs/libgomp/Offload-Target-Specifics.html and
add there the nvptx and gcn specific memory-allocation handling.

* * *

I think the following is not ideal in the pinning-is-not-supported case:

> @@ -434,10 +435,6 @@ omp_init_allocator (omp_memspace_handle_t
> memspace, int ntraits, } #endif
>
> -  /* No support for this so far.  */
> -  if (data.pinned)
> -    return omp_null_allocator;
> - ret = gomp_malloc (sizeof (struct omp_allocator_data));
> *ret = data;
> #ifndef HAVE_SYNC_BUILTINS

which continues as:
   gomp_mutex_init (&ret->lock);
#endif
   return (omp_allocator_handle_t) ret;
}


Therefore:

This code will always return a handle, even if pinning is not supported.
I had expected that the following happens:

"Otherwise if an allocator based on the requirements cannot be created
then the special omp_null_allocator handle is returned."

Using this allocator on a system where libgomp does not support pinning
will always fail with the fallback, which could be either of:

default_mem_fb (= omp_atv_default), null_fb, abort_fb, allocator_fb (+ fb_data).

Thus, the current code kind of works if the fallback is (explicitly or implicitly)
omp_atv_default or (explicitly) default_mem_fb – but otherwise, allocations will
always fail, most prominently with "abort_fb".

* * *
> The following definitions (ab)use comma operators to avoid unused
> variable errors. */ #ifndef MEMSPACE_ALLOC -#define
> MEMSPACE_ALLOC(MEMSPACE, SIZE) \ - malloc (((void)(MEMSPACE), (SIZE)))
> +#define MEMSPACE_ALLOC(MEMSPACE, SIZE, PIN) \ + (PIN ? NULL : malloc
> (((void)(MEMSPACE), (SIZE))))

I wonder whether the comment should note something like: All of the
following will return NULL or are a no-op when pinning is enabled
(unless overridden).

And the following looks odd:

> +#define MEMSPACE_FREE(MEMSPACE, ADDR, SIZE, PIN) \
> +  (PIN ? NULL :  free (((void)(MEMSPACE), (void)(SIZE), (ADDR))))
> #endif

Contrary to the other functions that return a value (pointer), 'free' "returns" 'void'.

And, indeed, the compiler might complain:

test.c:5:14: warning: ISO C forbids conditional expr with only one void side [-Wpedantic]
     5 |   pin ? NULL : free(p);
       |              ^

While (void)NULL works, I think the simplest is to just use an 'if (pin) free(...)'.

* * *
> +linux_memspace_alloc (omp_memspace_handle_t memspace, size_t size,
> int pin) +{ + (void)memspace; + + if (pin) + { + void *addr = mmap
> (NULL, size, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS,
> -1, 0);


Maybe add a comment noting that mmap returns nullified memory – as required for the calloc call.

(The linux man page states for MAP_ANONYMOUS: "The mapping ...; its contents are initialized to zero."
while POSIX has: "The system shall always zero-fill any partial page at the end of an object.", which
should be all in case of addr = NULL.)

> +      if (mlock (addr, size))
> +     { +       gomp_debug (0, "libgomp: failed to pin memory (ulimit too low?)\n");

I wonder whether the size should be included in the output - it might help to debug to know
whether "just" 1 kiB or 20 GB were tried to be pinned.

For the comment, I wonder whether it should mention RLIMIT_MEMLOCK or 'lockable memory' instead
or in addition to ulimit to be clearer.
(csh uses 'limit' instead of ulimit, but POSIX has both as function and as shell (sh, bash) ulimit,
i.e. using 'ulimit' is fine. Albeit 'ulimit()' has been deprecated in favour of {s,g}etrlimit().)

* * *

> +linux_memspace_realloc (omp_memspace_handle_t memspace, void *addr,
>  +                    size_t oldsize, size_t size, int oldpin, int pin)

(I was wondering about some corner cases, but I think the caller, i.e. omp_realloc,
already takes care of those.)

* * *

Regarding the testcases:

I think those are fine for __linux__ with only very minor assumptions (like that procfs is mounted).

However, for !defined(__linux__) it seems as if there are issues:

* * *

Regarding alloc-pinned-3.c:

> omp_allocator_handle_t allocator1 = omp_init_allocator
> (omp_default_mem_space, 2, traits1);

The code assumes that the return value cannot be omp_null_allocator.
But if no pinning is supported, it will be NULL. (Requested change from above.)

> /* Ensure that the limit is smaller than the allocation. */
> set_pin_limit (SIZE / 2); ... // Should fail void *p = omp_alloc
> (SIZE, allocator1); if (p) abort ();


The 'set_pin_limit ()' call is a no-op except on Linux. Thus, the allocation may fail or not,
depending whether the SIZE of ~39 MiB will is accepted or not.

Given that my Linux defaults to 4,075,856 kbytes and, for non-Linux, SIZE = 40,000 kiB,
it looks as if it might well work on larger systems - returning a non-null pointer.

> // Should fall back p = omp_alloc (SIZE, allocator2); if (!p) abort ();


How about calling 'omp_free' to avoid leaking + to check that the right 'free'
is called?

(multiple times)

* * *

Regarding alloc-pinned-1.c:

> #ifdef __linux__ ... #else #define PAGE_SIZE 1 /* unknown */ ...
> #endif ... /* Allocate at least a page each time, allowing space for
> overhead, but stay within the ulimit. */ const int SIZE = PAGE_SIZE -
> 128;

Are you sure that a SIZE of (1 - 128) = -127 is a good size?


For Linux:

> #define CHECK_SIZE(SIZE) { \ struct rlimit limit; \ if (getrlimit
> (RLIMIT_MEMLOCK, &limit) \ || limit.rlim_cur <= SIZE) \ fprintf
> (stderr, "unsufficient lockable memory; please increase ulimit\n"); \ }

I wonder whether you shouldn't add an 'abort()' here.

Otherwise, it will simply proceed - init_allocator will likely work, omp_alloc as
well due to the fallback and then:

   // Sanity check
   if (get_pinned_mem () != 0)
     abort ();

will fail. That works as well, but is somewhat odd.


Otherwise, as remarked above, you need to handle omp_init_allocator returning
omp_null_allocator (possibly aborting when __linux__ as we know that it should be
supported there.)


> int amount = get_pinned_mem (); if (amount == 0) abort ();

This one will always fail - except for __linux__, independent whether pinning worked or not.

(Three times)



Otherwise, it looks good to me.

Tobias

-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 5/6] libgomp, nvptx: Cuda pinned memory
  2023-08-23 14:14 ` [PATCH v2 5/6] libgomp, nvptx: Cuda pinned memory Andrew Stubbs
@ 2023-11-22 17:07   ` Tobias Burnus
  2023-12-07 13:43     ` Andrew Stubbs
  0 siblings, 1 reply; 15+ messages in thread
From: Tobias Burnus @ 2023-11-22 17:07 UTC (permalink / raw)
  To: Andrew Stubbs, gcc-patches

(I have not fully thought about the 2/6, 3/6 and 4/6 patches, but I
think except for some patch apply issues, 1/6 + this 5/6 can be both
committed without needing 2-4.)

On 23.08.23 16:14, Andrew Stubbs wrote:
> Use Cuda to pin memory, instead of Linux mlock, when available.
>
> There are two advantages: firstly, this gives a significant speed boost for
> NVPTX offloading, and secondly, it side-steps the usual OS ulimit/rlimit
> setting.
I think both the ulimit issue for the 1/6 patch and the non-issue for
this variant should/could be mentioned in libgomp.texi (in the Memory
Management section and in the nvptx section, respectively.)
> The design adds a device independent plugin API for allocating pinned memory,
> and then implements it for NVPTX.  At present, the other supported devices do
> not have equivalent capabilities (or requirements).

Note before: Starting with TR11 alias OpenMP 6.0, OpenMP supports handling
multiple devices for allocation. It seems as if after using:

   my_memspace = omp_get_device_and_host_memspace( 5 , omp_default_mem_space)
   my_alloc = omp_init_allocator (my_memspace, my_traits_with_pinning);

The pinning should be done via device '5' is possible.

  * * *

However, I believe that it shouldn't really matter for now, given that CUDA
has no special handling of NUMA hierarchy on the host nor for specific devices
and GCN has none.

It only becomes interesting if mmap/mlock memory is (measurably) faster than
CUDA allocated memory when accessed from the host or, for USM, from GCN.

* * *

Let's start with the patch itself:
> --- a/libgomp/target.c +++ b/libgomp/target.c ... +static struct
> gomp_device_descr * +get_device_for_page_locked (void) +{ + gomp_debug
> (0, "%s\n", + __FUNCTION__); + + struct gomp_device_descr *device;
> +#ifdef HAVE_SYNC_BUILTINS + device + = __atomic_load_n
> (&device_for_page_locked, MEMMODEL_RELAXED); + if (device == (void *)
> -1) + { + gomp_debug (0, " init\n"); + + gomp_init_targets_once (); +
> + device = NULL; + for (int i = 0; i < num_devices; ++i)

Given that this function just sets a single variable based on whether the
page_locked_host_alloc_func function pointer exists, wouldn't it be much
simpler to just do all this handling in   gomp_target_init  ?

> + for (int i = 0; i < num_devices; ++i) ... + /* We consider only the
> first device of potentially several of the + same type as this
> functionality is not specific to an individual + offloading device,
> but instead relates to the host-side + implementation of the
> respective offloading implementation. */ + if (devices[i].target_id !=
> 0) + continue; + + if (!devices[i].page_locked_host_alloc_func) +
> continue; ... + if (device) + gomp_fatal ("Unclear how %s and %s
> libgomp plugins may" + " simultaneously provide functionality" + " for
> page-locked memory", + device->name, devices[i].name); + else + device
> = &devices[i];

I find this a bit inconsistent: If - let's say - GCN does not not provide its
own pinning, the code assumes that CUDA pinning is just fine.  However, if both
support it, CUDA pinning suddenly is not fine for GCN.

Additionally, all wording suggests that it does not matter for CUDA for which
device access we want to optimize the pinning. But the code above also fails if
I have a system with two Nvidia cards.  From the wording, it sounds as if just
checking whether the  device->type  is different would do.


But all in all, I wonder whether it wouldn't be much simpler to state something
like the following (where applicable):

If first device that provided pinning support is used; the assumption is that
all other devices and the host can access this memory without measurable
performance penalty compared to a normal page lock and that having multiple
device types or host/device NUMA aware pinning support in the plugin is not
available.
NOTE: For OpenMP 6.0's OMP_AVAILABLE_DEVICES environment variable, device-set
memory spaces this might need to be revisited.

(The note is only meant for the *.c code / this review, the first sentence up
to the ';' should do in some way into libgomp.texi as well.)

And document in libgomp.texi that the first device for which device-specific
host-memory pinning support is available is used → to be added in
https://gcc.gnu.org/onlinedocs/libgomp/Memory-allocation.html
The nvidia specific part - i.e. that it is supported and possibly more details -
can then be added to: https://gcc.gnu.org/onlinedocs/libgomp/nvptx.html

I think the @ref added to 'Offload-Target Specifics' for the third time for
this patch will be sufficient - if not, add some referring words as well.


  * * *

> +gomp_page_locked_host_alloc (void **ptr, size_t size) +{ + gomp_debug
> (0, "%s: ptr=%p, size=%llu\n", + __FUNCTION__, ptr, (unsigned long
> long) size); + + struct gomp_device_descr *device =
> get_device_for_page_locked ();

With the proposed changes above, we could just call
   device_for_page_locked
and then access the global variable nonatomically.

BTW: I think the global variable needs a _mem(ory) suffix, I find
   device_for_page_locked
somewhat unclear. I also wonder whether ..._pinned_mem(ory) is clearer
than ..._paged_locked_mem(ory), but I think either works.

  * * *

I was wondering whether there should be a very noisy fail (error printed +
gomp_fatal by the caller) or not:

> +++ b/libgomp/plugin/plugin-nvptx.c
> +GOMP_OFFLOAD_page_locked_host_alloc (void **ptr, size_t size) ... + r
> = CUDA_CALL_NOCHECK (cuMemHostAlloc, ptr, size, flags); + if (r ==
> CUDA_ERROR_OUT_OF_MEMORY) + *ptr = NULL; + else if (r != CUDA_SUCCESS)
> + { + GOMP_PLUGIN_error ("cuMemHostAlloc error: %s", cuda_error (r));

The possible return values are:
   CUDA_SUCCESS
   CUDA_ERROR_DEINITIALIZED
   CUDA_ERROR_NOT_INITIALIZED
   CUDA_ERROR_INVALID_CONTEXT
   CUDA_ERROR_INVALID_VALUE
   CUDA_ERROR_OUT_OF_MEMORY

=> I think the current handling is okay - if and only if 'size = 0' is
handled gracefully by returning 0.  (Either by this function or by all its
callers.) For omp_alloc, OpenMP states:
     "If size is 0, omp_alloc and omp_aligned_alloc will return NULL."

(I have the feeling that CUDA might return CUDA_ERROR_INVALID_VALUE in
that case.)

* * *

Side remark:
    omp_alloc (my_alloc, 0)
should return NULL - and not use any fallback. I am pretty sure that is explicitly
handled in omp_alloc. I think omp_alloc is also called in all relevant cases for other
codepaths, except for omp_realloc which has/needs some extra handling, but I think that's
done in the generic code in libgomp/allocator.c

* * *


> --- a/libgomp/config/linux/allocator.c +++
> b/libgomp/config/linux/allocator.c ... -linux_memspace_alloc
> (omp_memspace_handle_t memspace, size_t size, int pin)
> +linux_memspace_alloc (omp_memspace_handle_t memspace, size_t size,
> int pin, + bool init0) { ... else - return malloc (size); + addr =
> malloc (size); + + if (addr && init0) + { + gomp_debug (0, "
> init0\n"); + memset (addr, 0, size); + }

This code looks very puzzling - I think it is beast to create an auxiliary function,
e.g., linux_memspace_alloc_pinned, and call that from both
   linux_memspace_alloc
and
   linux_memspace_calloc

That avoids the 'if (pin)' in the aux function and the odd case that (for 'if(pin)')
calloc calls into a function that at the end might (for '!pin') call malloc instead
of calloc - followed by an init0.

* * *

> --- a/libgomp/testsuite/libgomp.c/alloc-pinned-1.c +++
> b/libgomp/testsuite/libgomp.c/alloc-pinned-1.c +/* {
> dg-additional-options -DOFFLOAD_DEVICE_NVPTX { target
> offload_device_nvptx } } */

I think this is sufficiently fine. But for completeness, I think this is not 100%
reliable. The check is for the default device, which is by default the first available
device.  For the tested-for feature, it is sufficient if *any* available device is an
nvptx device. - If you have a GCN and a Nvptx GPU and the GCN one comes first, CUDA's
allocator will be used - but the OFFLOAD_DEVICE_NVPTX is unset. Likewise for GCN + NVPTX
and some OMP_DEFAULT_DEVICE=... trickery.

I think a host with both nvptx + gcn is rare - and if it becomes an issue, we just need
to extend the effective-target checks.

* * *

> @@ -67,10 +69,15 @@ verify0 (char *p, size_t s) int main () { +#ifdef
> OFFLOAD_DEVICE_NVPTX + /* Go big or go home. */ + const int SIZE = 40
> * 1024 * 1024; +#else

I wonder whether it makes sense to add the comment that the 'ulimit' does not
affect nvptx's pinned-memory allocator, just for completeness and as service for
the test-case reader.

Otherwise, I have not spotted anything.

Tobias

PS: Next comes "[PATCH v2 6/6] libgomp: fine-grained pinned memory allocator" to
complete the library-only bit of this patch series.

-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 1/6] libgomp: basic pinned memory on Linux
  2023-11-22 14:26   ` Tobias Burnus
@ 2023-11-29 17:05     ` Andrew Stubbs
  0 siblings, 0 replies; 15+ messages in thread
From: Andrew Stubbs @ 2023-11-29 17:05 UTC (permalink / raw)
  To: Tobias Burnus, gcc-patches; +Cc: Jakub Jelinek

On 22/11/2023 14:26, Tobias Burnus wrote:
> Hi Andrew,
> 
> Side remark:
> 
>> -#define MEMSPACE_CALLOC(MEMSPACE, SIZE) \ - calloc (1,
>> (((void)(MEMSPACE), (SIZE))))
> 
> This fits a bit more to previous patch, but I wonder whether that should
> use (MEMSPACE, NMEMB, SIZE) instead - to fit to the actual calloc 
> arguments.
> 
> I think the main/only difference between SIZE and NMEMB and SIZE is that
> "If the multiplication of nmemb and size would result in integer overflow,
> then calloc() returns an error." (Linux manpage)
> 
> However, while this wording seems to be neither in POSIX nor in the OpenMP
> spec. There was some alignment discussion at https://gcc.gnu.org/PR112364
> regarding whether C (since C23) has a different alignment for
> calloc(1, n) vs. calloc(n,1) but Joseph believes it doen't.
> 
> Thus, this is more bikesheding than making a real difference.

[Addressing this point separately to the others....]

The size has already been calculated, aligned, and padded, before we get 
to calling MEMSPACE_CALLOC. I don't think we can revert to "nmemb, size" 
without breaking that.

Andrew

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 5/6] libgomp, nvptx: Cuda pinned memory
  2023-11-22 17:07   ` Tobias Burnus
@ 2023-12-07 13:43     ` Andrew Stubbs
  2023-12-08 14:09       ` Thomas Schwinge
  0 siblings, 1 reply; 15+ messages in thread
From: Andrew Stubbs @ 2023-12-07 13:43 UTC (permalink / raw)
  To: Tobias Burnus, gcc-patches, Thomas Schwinge

@Thomas, there are questions for you below....

On 22/11/2023 17:07, Tobias Burnus wrote:
> Note before: Starting with TR11 alias OpenMP 6.0, OpenMP supports handling
> multiple devices for allocation. It seems as if after using:
> 
>    my_memspace = omp_get_device_and_host_memspace( 5 , 
> omp_default_mem_space)
>    my_alloc = omp_init_allocator (my_memspace, my_traits_with_pinning);
> 
> The pinning should be done via device '5' is possible.

This patch is intended to get us to 5.0. These are future features for 
future patches.

> However, I believe that it shouldn't really matter for now, given that CUDA
> has no special handling of NUMA hierarchy on the host nor for specific 
> devices
> and GCN has none.
> 
> It only becomes interesting if mmap/mlock memory is (measurably) faster 
> than
> CUDA allocated memory when accessed from the host or, for USM, from GCN.

I don't believe there's any issue here (yet).

> Let's start with the patch itself:
>> --- a/libgomp/target.c
>> +++ b/libgomp/target.c
>> ...
>> +static struct gomp_device_descr *
>> +get_device_for_page_locked (void)
>> +{
>> + gomp_debug (0, "%s\n",
>> +             __FUNCTION__);
>> +
>> + struct gomp_device_descr *device;
>> +#ifdef HAVE_SYNC_BUILTINS
>> + device
>> +   = __atomic_load_n (&device_for_page_locked, MEMMODEL_RELAXED);
>> + if (device == (void *) -1)
>> +   {
>> +     gomp_debug (0, " init\n");
>> +
>> +     gomp_init_targets_once ();
>> +
>> +     device = NULL;
>> +     for (int i = 0; i < num_devices; ++i)
> 
> Given that this function just sets a single variable based on whether the
> page_locked_host_alloc_func function pointer exists, wouldn't it be much
> simpler to just do all this handling in   gomp_target_init  ?

@Thomas, care to comment on this?

>> + for (int i = 0; i < num_devices; ++i)
>> ...
>> +    /* We consider only the first device of potentially several of the
>> +       same type as this functionality is not specific to an individual
>> +       offloading device, but instead relates to the host-side
>> +       implementation of the respective offloading implementation. */
>> +    if (devices[i].target_id != 0)
>> +      continue;
>> +
>> +    if (!devices[i].page_locked_host_alloc_func)
>> +      continue;
>> ...
>> +    if (device)
>> +      gomp_fatal ("Unclear how %s and %s libgomp plugins may"
>> +                  " simultaneously provide functionality"
>> +                  " for page-locked memory",
>> +                  device->name, devices[i].name);
>> +    else
>> +      device = &devices[i];
> 
> I find this a bit inconsistent: If - let's say - GCN does not not 
> provide its
> own pinning, the code assumes that CUDA pinning is just fine.  However, 
> if both
> support it, CUDA pinning suddenly is not fine for GCN.

I think it means that we need to revisit this code if that situation 
ever occurs. Again, @Thomas?

> Additionally, all wording suggests that it does not matter for CUDA for 
> which
> device access we want to optimize the pinning. But the code above also 
> fails if
> I have a system with two Nvidia cards.  From the wording, it sounds as 
> if just
> checking whether the  device->type  is different would do.
> 
> 
> But all in all, I wonder whether it wouldn't be much simpler to state 
> something
> like the following (where applicable):
> 
> If first device that provided pinning support is used; the assumption is 
> that
> all other devices and the host can access this memory without measurable
> performance penalty compared to a normal page lock and that having multiple
> device types or host/device NUMA aware pinning support in the plugin is not
> available.
> NOTE: For OpenMP 6.0's OMP_AVAILABLE_DEVICES environment variable, 
> device-set
> memory spaces this might need to be revisited.

This seems reasonable to me, until the user can specify.

(I'm going to go look at the other review points now....)

Andrew

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 5/6] libgomp, nvptx: Cuda pinned memory
  2023-12-07 13:43     ` Andrew Stubbs
@ 2023-12-08 14:09       ` Thomas Schwinge
  2023-12-08 16:44         ` Tobias Burnus
  0 siblings, 1 reply; 15+ messages in thread
From: Thomas Schwinge @ 2023-12-08 14:09 UTC (permalink / raw)
  To: Andrew Stubbs, Tobias Burnus; +Cc: gcc-patches

Hi!

On 2023-12-07T13:43:17+0000, Andrew Stubbs <ams@codesourcery.com> wrote:
> @Thomas, there are questions for you below....

It's been a while that I've been working on this; I'll try to produce
some coherent answers now.

> On 22/11/2023 17:07, Tobias Burnus wrote:
>> Let's start with the patch itself:
>>> --- a/libgomp/target.c
>>> +++ b/libgomp/target.c
>>> ...
>>> +static struct gomp_device_descr *
>>> +get_device_for_page_locked (void)
>>> +{
>>> + gomp_debug (0, "%s\n",
>>> +             __FUNCTION__);
>>> +
>>> + struct gomp_device_descr *device;
>>> +#ifdef HAVE_SYNC_BUILTINS
>>> + device
>>> +   = __atomic_load_n (&device_for_page_locked, MEMMODEL_RELAXED);
>>> + if (device == (void *) -1)
>>> +   {
>>> +     gomp_debug (0, " init\n");
>>> +
>>> +     gomp_init_targets_once ();
>>> +
>>> +     device = NULL;
>>> +     for (int i = 0; i < num_devices; ++i)
>>
>> Given that this function just sets a single variable based on whether the
>> page_locked_host_alloc_func function pointer exists, wouldn't it be much
>> simpler to just do all this handling in   gomp_target_init  ?
>
> @Thomas, care to comment on this?

From what I remember, we cannot assume that 'gomp_target_init' has
already been done when we get here; therefore 'gomp_init_targets_once' is
being called here.  We may get to 'get_device_for_page_locked' via
host-side OpenMP, in code that doesn't contain any OpenMP 'target'
offloading things.  Therefore, this was (a) necessary to make that work,
and (b) did seem to be a useful abstraction to me.

>>> + for (int i = 0; i < num_devices; ++i)
>>> ...
>>> +    /* We consider only the first device of potentially several of the
>>> +       same type as this functionality is not specific to an individual
>>> +       offloading device, but instead relates to the host-side
>>> +       implementation of the respective offloading implementation. */
>>> +    if (devices[i].target_id != 0)
>>> +      continue;
>>> +
>>> +    if (!devices[i].page_locked_host_alloc_func)
>>> +      continue;
>>> ...
>>> +    if (device)
>>> +      gomp_fatal ("Unclear how %s and %s libgomp plugins may"
>>> +                  " simultaneously provide functionality"
>>> +                  " for page-locked memory",
>>> +                  device->name, devices[i].name);
>>> +    else
>>> +      device = &devices[i];
>>
>> I find this a bit inconsistent: If - let's say - GCN does not not
>> provide its
>> own pinning, the code assumes that CUDA pinning is just fine.  However,
>> if both
>> support it, CUDA pinning suddenly is not fine for GCN.
>
> I think it means that we need to revisit this code if that situation
> ever occurs. Again, @Thomas?

That's correct.  As you know, I don't like doing half-baked things.  ;-)
However, this did seem like a useful stepping-stone to me, to get such a
thing implemented at all; we do understand that this won't handle all
(future) cases, thus the 'gomp_fatal' to catch that loudly.

Once we are in the situation where we have multiple ways of allocating
large amounts of pinned memory, we'll have to see how to deal with that.
(May, of course, already now work out how conceptually that should be
done, possibly via OpenMP committee/specification work, if necessary?)
(As for the future implementation, maybe *allocate* via one device, and
then *register* the allocation with the other devices.)

>> Additionally, all wording suggests that it does not matter for CUDA for
>> which
>> device access we want to optimize the pinning. But the code above also
>> fails if
>> I have a system with two Nvidia cards.

Why/how does the code fail in that case?  Assuming I understood the
question correctly, the 'if (devices[i].target_id != 0) continue;' is
meant to handle that case.

>> From the wording, it sounds as
>> if just
>> checking whether the  device->type  is different would do.

Maybe, but I'm not sure I follow what exactly you mean.

>> But all in all, I wonder whether it wouldn't be much simpler to state
>> something
>> like the following (where applicable):
>>
>> If first device that provided pinning support is used; the assumption is
>> that
>> all other devices

"of the same kind" or also "of different kinds"?

>> and the host can access this memory without measurable
>> performance penalty compared to a normal page lock and that having multiple
>> device types or host/device NUMA aware pinning support in the plugin is not
>> available.

If I understood you correctly, that, however, is not correct: if you
(hypothetically) allocate pinned memory via GCN (or even the small amount
you get via the host), then yes, a nvptx device will be able to access
it, but it won't see the performance gains that you'd get if you had
allocated via nvptx.  (You'll need to register existing memory regions
with the nvptx device/CUDA, if I offhand remember correctly, which is
subject to later work.)


Hopefully that did help?


Grüße
 Thomas


>> NOTE: For OpenMP 6.0's OMP_AVAILABLE_DEVICES environment variable,
>> device-set
>> memory spaces this might need to be revisited.
>
> This seems reasonable to me, until the user can specify.
>
> (I'm going to go look at the other review points now....)
>
> Andrew
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 5/6] libgomp, nvptx: Cuda pinned memory
  2023-12-08 14:09       ` Thomas Schwinge
@ 2023-12-08 16:44         ` Tobias Burnus
  2023-12-11 14:31           ` Thomas Schwinge
  0 siblings, 1 reply; 15+ messages in thread
From: Tobias Burnus @ 2023-12-08 16:44 UTC (permalink / raw)
  To: Thomas Schwinge, Andrew Stubbs, Tobias Burnus; +Cc: gcc-patches

On 08.12.23 15:09, Thomas Schwinge wrote:
>> On 22/11/2023 17:07, Tobias Burnus wrote:
>>> Let's start with the patch itself:
>>>> --- a/libgomp/target.c
>>>> +++ b/libgomp/target.c
>>>> ...
>>>> +static struct gomp_device_descr *
>>>> +get_device_for_page_locked (void)
>>>> +{
>>>> + gomp_debug (0, "%s\n",
>>>> +             __FUNCTION__);
>>>> +
>>>> + struct gomp_device_descr *device;
>>>> +#ifdef HAVE_SYNC_BUILTINS
>>>> + device
>>>> +   = __atomic_load_n (&device_for_page_locked, MEMMODEL_RELAXED);
>>>> + if (device == (void *) -1)
>>>> +   {
>>>> +     gomp_debug (0, " init\n");
>>>> +
>>>> +     gomp_init_targets_once ();
>>>> +
>>>> +     device = NULL;
>>>> +     for (int i = 0; i < num_devices; ++i)
>>> Given that this function just sets a single variable based on whether the
>>> page_locked_host_alloc_func function pointer exists, wouldn't it be much
>>> simpler to just do all this handling in   gomp_target_init  ?
>> @Thomas, care to comment on this?
>  From what I remember, we cannot assume that 'gomp_target_init' has
> already been done when we get here; therefore 'gomp_init_targets_once' is
> being called here.  We may get to 'get_device_for_page_locked' via
> host-side OpenMP, in code that doesn't contain any OpenMP 'target'
> offloading things.  Therefore, this was (a) necessary to make that work,
> and (b) did seem to be a useful abstraction to me.

I am not questioning the "gomp_init_targets_once ();" but I am wounding
whether only 'gomp_init_targets_once()' should remain without the
locking + loading dance - and then just set that single variable inside
gomp_target_init.

If you reach here w/o target set up, the "gomp_init_targets_once ();"
would ensure it gets initialized with all the other code inside
gomp_target_init.

And if gomp_target_init() was called before, gomp_init_targets_once()
will just return without doing anything and your are also fine.

Tobias

-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 5/6] libgomp, nvptx: Cuda pinned memory
  2023-12-08 16:44         ` Tobias Burnus
@ 2023-12-11 14:31           ` Thomas Schwinge
  2023-12-11 16:05             ` Tobias Burnus
  0 siblings, 1 reply; 15+ messages in thread
From: Thomas Schwinge @ 2023-12-11 14:31 UTC (permalink / raw)
  To: Tobias Burnus, Andrew Stubbs; +Cc: gcc-patches

Hi!

On 2023-12-08T17:44:14+0100, Tobias Burnus <tobias@codesourcery.com> wrote:
> On 08.12.23 15:09, Thomas Schwinge wrote:
>>> On 22/11/2023 17:07, Tobias Burnus wrote:
>>>> Let's start with the patch itself:
>>>>> --- a/libgomp/target.c
>>>>> +++ b/libgomp/target.c
>>>>> ...
>>>>> +static struct gomp_device_descr *
>>>>> +get_device_for_page_locked (void)
>>>>> +{
>>>>> + gomp_debug (0, "%s\n",
>>>>> +             __FUNCTION__);
>>>>> +
>>>>> + struct gomp_device_descr *device;
>>>>> +#ifdef HAVE_SYNC_BUILTINS
>>>>> + device
>>>>> +   = __atomic_load_n (&device_for_page_locked, MEMMODEL_RELAXED);
>>>>> + if (device == (void *) -1)
>>>>> +   {
>>>>> +     gomp_debug (0, " init\n");
>>>>> +
>>>>> +     gomp_init_targets_once ();
>>>>> +
>>>>> +     device = NULL;
>>>>> +     for (int i = 0; i < num_devices; ++i)
>>>> Given that this function just sets a single variable based on whether the
>>>> page_locked_host_alloc_func function pointer exists, wouldn't it be much
>>>> simpler to just do all this handling in   gomp_target_init  ?
>>> @Thomas, care to comment on this?
>>  From what I remember, we cannot assume that 'gomp_target_init' has
>> already been done when we get here; therefore 'gomp_init_targets_once' is
>> being called here.  We may get to 'get_device_for_page_locked' via
>> host-side OpenMP, in code that doesn't contain any OpenMP 'target'
>> offloading things.  Therefore, this was (a) necessary to make that work,
>> and (b) did seem to be a useful abstraction to me.
>
> I am not questioning the "gomp_init_targets_once ();" but I am wounding
> whether only 'gomp_init_targets_once()' should remain without the
> locking + loading dance - and then just set that single variable inside
> gomp_target_init.

Ah, I see, thanks.

> If you reach here w/o target set up, the "gomp_init_targets_once ();"
> would ensure it gets initialized with all the other code inside
> gomp_target_init.
>
> And if gomp_target_init() was called before, gomp_init_targets_once()
> will just return without doing anything and your are also fine.

Yes, I suppose we could do it that way.  'get_device_for_page_locked'
could then, after 'gomp_init_targets_once', unconditionally return
'device_for_page_locked' (even without '__atomic_load', right?).
A disadvantage is that the setup of 'device_for_page_locked' (in
'gomp_target_init') and use of it (in 'get_device_for_page_locked') is
then split apart.  I guess I don't have a strong opinion on that one.
;-)


Grüße
 Thomas
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 5/6] libgomp, nvptx: Cuda pinned memory
  2023-12-11 14:31           ` Thomas Schwinge
@ 2023-12-11 16:05             ` Tobias Burnus
  0 siblings, 0 replies; 15+ messages in thread
From: Tobias Burnus @ 2023-12-11 16:05 UTC (permalink / raw)
  To: Thomas Schwinge, Andrew Stubbs; +Cc: gcc-patches

On 11.12.23 15:31, Thomas Schwinge wrote:
> On 2023-12-08T17:44:14+0100, Tobias Burnus <tobias@codesourcery.com> wrote:
>> On 08.12.23 15:09, Thomas Schwinge wrote:
>>>> On 22/11/2023 17:07, Tobias Burnus wrote:
>>>>> Let's start with the patch itself:
>>>>>> --- a/libgomp/target.c
>>>>>> +++ b/libgomp/target.c
>>>>>> ...
>>>>>> +static struct gomp_device_descr *
>>>>>> +get_device_for_page_locked (void)
>>>>>> +{
>>>>>> + gomp_debug (0, "%s\n",
>>>>>> +             __FUNCTION__);
>>>>>> +
>>>>>> + struct gomp_device_descr *device;
>>>>>> +#ifdef HAVE_SYNC_BUILTINS
>>>>>> + device
>>>>>> +   = __atomic_load_n (&device_for_page_locked, MEMMODEL_RELAXED);
>>>>>> + if (device == (void *) -1)
>>>>>> +   {
>>>>>> +     gomp_debug (0, " init\n");
>>>>>> +
>>>>>> +     gomp_init_targets_once ();
>>>>>> +
>>>>>> +     device = NULL;
>>>>>> +     for (int i = 0; i < num_devices; ++i)
>>>>> Given that this function just sets a single variable based on whether the
>>>>> page_locked_host_alloc_func function pointer exists, wouldn't it be much
>>>>> simpler to just do all this handling in   gomp_target_init  ?
>>>> @Thomas, care to comment on this?
>>>   From what I remember, we cannot assume that 'gomp_target_init' has
>>> already been done when we get here; therefore 'gomp_init_targets_once' is
>>> being called here.  We may get to 'get_device_for_page_locked' via
>>> host-side OpenMP, in code that doesn't contain any OpenMP 'target'
>>> offloading things.  Therefore, this was (a) necessary to make that work,
>>> and (b) did seem to be a useful abstraction to me.
>> I am not questioning the "gomp_init_targets_once ();" but I am wounding
>> whether only 'gomp_init_targets_once()' should remain without the
>> locking + loading dance - and then just set that single variable inside
>> gomp_target_init.
> Ah, I see, thanks.
>
>> If you reach here w/o target set up, the "gomp_init_targets_once ();"
>> would ensure it gets initialized with all the other code inside
>> gomp_target_init.
>>
>> And if gomp_target_init() was called before, gomp_init_targets_once()
>> will just return without doing anything and your are also fine.
> Yes, I suppose we could do it that way.  'get_device_for_page_locked'
> could then, after 'gomp_init_targets_once', unconditionally return
> 'device_for_page_locked' (even without '__atomic_load', right?).
Yes, that was my idea.
> A disadvantage is that the setup of 'device_for_page_locked' (in
> 'gomp_target_init') and use of it (in 'get_device_for_page_locked') is
> then split apart.  I guess I don't have a strong opinion on that one.
> ;-)

But pro is that it avoids the #ifdef HAVE_SYNC_BUILTINS, avoiding a "-1"
initialization of using_device_for_page_locked, atomic loads all over
the place etc.

Thus, I prefer this option – but I also don't have a strong opinion,
either.

Tobias

-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2023-12-11 16:05 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-23 14:14 [PATCH v2 0/6] libgomp: OpenMP pinned memory omp_alloc Andrew Stubbs
2023-08-23 14:14 ` [PATCH v2 1/6] libgomp: basic pinned memory on Linux Andrew Stubbs
2023-11-22 14:26   ` Tobias Burnus
2023-11-29 17:05     ` Andrew Stubbs
2023-08-23 14:14 ` [PATCH v2 2/6] libgomp, openmp: Add ompx_pinned_mem_alloc Andrew Stubbs
2023-08-23 14:14 ` [PATCH v2 3/6] openmp: Add -foffload-memory Andrew Stubbs
2023-08-23 14:14 ` [PATCH v2 4/6] openmp: -foffload-memory=pinned Andrew Stubbs
2023-08-23 14:14 ` [PATCH v2 5/6] libgomp, nvptx: Cuda pinned memory Andrew Stubbs
2023-11-22 17:07   ` Tobias Burnus
2023-12-07 13:43     ` Andrew Stubbs
2023-12-08 14:09       ` Thomas Schwinge
2023-12-08 16:44         ` Tobias Burnus
2023-12-11 14:31           ` Thomas Schwinge
2023-12-11 16:05             ` Tobias Burnus
2023-08-23 14:14 ` [PATCH v2 6/6] libgomp: fine-grained pinned memory allocator Andrew Stubbs

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).