Silence valgrind about pg_numa_touch_mem_if_required

When querying NUMA status of pages in shared memory, we need to touch
the memory first to get valid results. This may trigger valgrind
reports, because some of the memory (e.g. unpinned buffers) may be
marked as noaccess.

Solved by adding a valgrind suppresion. An alternative would be to
adjust the access/noaccess status before touching the memory, but that
seems far too invasive. It would require all those places to have
detailed knowledge of what the shared memory stores.

The pg_numa_touch_mem_if_required() macro is replaced with a function.
Macros are invisible to suppressions, so it'd have to suppress reports
for the caller - e.g. pg_get_shmem_allocations_numa(). So we'd suppress
reports for the whole function, and that seems to heavy-handed. It might
easily hide other valid issues.

Reviewed-by: Christoph Berg <myon@debian.org>
Reviewed-by: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://postgr.es/m/aEtDozLmtZddARdB@msg.df7cb.de
Backpatch-through: 18

diff --git a/contrib/pg_buffercache/pg_buffercache_pages.c b/contrib/pg_buffercache/pg_buffercache_pages.c
index 4b007f6e1b06a622559c7e14d83450afbcb54d4b..ae0291e6e96df2fc0a4530507022cce6c71875d5 100644 (file)
--- a/contrib/pg_buffercache/pg_buffercache_pages.c
+++ b/contrib/pg_buffercache/pg_buffercache_pages.c
@@ -320,7 +320,6 @@ pg_buffercache_numa_pages(PG_FUNCTION_ARGS)
                uint64          os_page_count;
                int                     pages_per_buffer;
                int                     max_entries;
-               volatile uint64 touch pg_attribute_unused();
                char       *startptr,
                                   *endptr;
 
@@ -375,7 +374,7 @@ pg_buffercache_numa_pages(PG_FUNCTION_ARGS)
 
                        /* Only need to touch memory once per backend process lifetime */
                        if (firstNumaTouch)
-                               pg_numa_touch_mem_if_required(touch, ptr);
+                               pg_numa_touch_mem_if_required(ptr);
                }
 
                Assert(idx == os_page_count);

diff --git a/src/backend/storage/ipc/shmem.c b/src/backend/storage/ipc/shmem.c
index c9ae3b45b76b11b9647348d5445511983b17526d..ca3656fc76f4300ab23b8d664dcaea38a45ed804 100644 (file)
--- a/src/backend/storage/ipc/shmem.c
+++ b/src/backend/storage/ipc/shmem.c
@@ -679,12 +679,10 @@ pg_get_shmem_allocations_numa(PG_FUNCTION_ARGS)
                 */
                for (i = 0; i < shm_ent_page_count; i++)
                {
-                       volatile uint64 touch pg_attribute_unused();
-
                        page_ptrs[i] = startptr + (i * os_page_size);
 
                        if (firstNumaTouch)
-                               pg_numa_touch_mem_if_required(touch, page_ptrs[i]);
+                               pg_numa_touch_mem_if_required(page_ptrs[i]);
 
                        CHECK_FOR_INTERRUPTS();
                }

diff --git a/src/include/port/pg_numa.h b/src/include/port/pg_numa.h
index 40f1d324dcfe275a8d922cc554792e197805c7c9..6c8b7103cc344ee18304a0256cdb98927a5e2f41 100644 (file)
--- a/src/include/port/pg_numa.h
+++ b/src/include/port/pg_numa.h
@@ -24,12 +24,16 @@ extern PGDLLIMPORT int pg_numa_get_max_node(void);
  * This is required on Linux, before pg_numa_query_pages() as we
  * need to page-fault before move_pages(2) syscall returns valid results.
  */
-#define pg_numa_touch_mem_if_required(ro_volatile_var, ptr) \
-       ro_volatile_var = *(volatile uint64 *) ptr
+static inline void
+pg_numa_touch_mem_if_required(void *ptr)
+{
+       volatile uint64 touch pg_attribute_unused();
+       touch = *(volatile uint64 *) ptr;
+}
 
 #else
 
-#define pg_numa_touch_mem_if_required(ro_volatile_var, ptr) \
+#define pg_numa_touch_mem_if_required(ptr) \
        do {} while(0)
 
 #endif

diff --git a/src/tools/valgrind.supp b/src/tools/valgrind.supp
index 7ea464c809417feafa918b2ae2034df9479913eb..2ad5b81526d3f7649882575660e7ccb40717c9f8 100644 (file)
--- a/src/tools/valgrind.supp
+++ b/src/tools/valgrind.supp
@@ -180,3 +180,17 @@
    Memcheck:Cond
    fun:PyObject_Realloc
 }
+
+# NUMA introspection requires touching memory first, and some of it may
+# be marked as noacess (e.g. unpinned buffers). So just ignore that.
+{
+   pg_numa_touch_mem_if_required
+   Memcheck:Addr4
+   fun:pg_numa_touch_mem_if_required
+}
+
+{
+   pg_numa_touch_mem_if_required
+   Memcheck:Addr8
+   fun:pg_numa_touch_mem_if_required
+}