On 4/8/25 16:59, Andres Freund wrote:
> Hi,
>
> On 2025-04-08 09:35:37 -0400, Andres Freund wrote:
>> On April 8, 2025 9:21:57 AM EDT, Tomas Vondra <to...@vondra.me> wrote:
>>> On 4/8/25 15:06, Andres Freund wrote:
>>>> On 2025-04-08 17:44:19 +0500, Kirill Reshke wrote:
>>>> I think it's not right that something in src/port defines an SQL callable
>>>> function. The set of headers for that pull in a lot of things.
>>>>
>>>> Since the file relies on things like GUCs, I suspect this should be in
>>>> src/backend/port or such instead.
>>>>
>>>
>>> Yeah, I think you're right, src/backend/port seems like a better place
>>> for this. I'll look into moving that in the evening.
>>
>> On a second look I wonder if just the SQL function and perhaps the page size
>> function should be moved. There are FE programs that could potentially
>> benefit from num a awareness (e.g. pgbench).
>
> I would move pg_numa_available() to something like
> src/backend/storage/ipc/shmem.c.
>
Makes sense, done in the attached patch.
> I wonder if pg_numa_get_pagesize() should loose the _numa_ in the name, it's
> not actually directly NUMA related? If it were pg_get_pagesize() it'd fit in
> with shmem.c or such.
>
True. It's true it's not really "NUMA page", but page size for the shmem
segment. So renamed to pg_get_shmem_pagesize() and moved to shmem.c,
same as pg_numa_available().
The rest of pg_numa.c is moved to src/backend/port
> Or we could just make it work in FE code by making this part
>
> Assert(IsUnderPostmaster);
> Assert(huge_pages_status != HUGE_PAGES_UNKNOWN);
>
> if (huge_pages_status == HUGE_PAGES_ON)
> GetHugePageSize(&os_page_size, NULL);
>
> #ifndef FRONTEND - we don't currently support using huge pages in FE programs
> after all. But querying the page size might still be useful.
>
I don't really like this. Why shouldn't the FE program simply call
sysconf(_SC_PAGESIZE)? It'd be just confusing if in backend it'd also
verify huge page status.
>
> Regardless of all of that, I don't think the include of fmgr.h in pg_numa.h is
> needed?
>
Right, that was left there after moving the prototype into the .c file.
regards
--
Tomas Vondra
diff --git a/contrib/pg_buffercache/pg_buffercache_pages.c b/contrib/pg_buffercache/pg_buffercache_pages.c
index c9ceba604b1..e1701bd56ef 100644
--- a/contrib/pg_buffercache/pg_buffercache_pages.c
+++ b/contrib/pg_buffercache/pg_buffercache_pages.c
@@ -343,7 +343,7 @@ pg_buffercache_numa_pages(PG_FUNCTION_ARGS)
* This information is needed before calling move_pages() for NUMA
* node id inquiry.
*/
- os_page_size = pg_numa_get_pagesize();
+ os_page_size = pg_get_shmem_pagesize();
/*
* The pages and block size is expected to be 2^k, so one divides the
diff --git a/src/backend/port/Makefile b/src/backend/port/Makefile
index 47338d99229..5dafbf7c0c0 100644
--- a/src/backend/port/Makefile
+++ b/src/backend/port/Makefile
@@ -24,6 +24,7 @@ include $(top_builddir)/src/Makefile.global
OBJS = \
$(TAS) \
atomics.o \
+ pg_numa.o \
pg_sema.o \
pg_shmem.o
diff --git a/src/backend/port/meson.build b/src/backend/port/meson.build
index 09d54e01d13..a9f7120aef4 100644
--- a/src/backend/port/meson.build
+++ b/src/backend/port/meson.build
@@ -2,6 +2,7 @@
backend_sources += files(
'atomics.c',
+ 'pg_numa.c',
)
diff --git a/src/port/pg_numa.c b/src/backend/port/pg_numa.c
similarity index 71%
rename from src/port/pg_numa.c
rename to src/backend/port/pg_numa.c
index 5e2523cf798..20be13f669d 100644
--- a/src/port/pg_numa.c
+++ b/src/backend/port/pg_numa.c
@@ -20,7 +20,6 @@
#include <windows.h>
#endif
-#include "fmgr.h"
#include "miscadmin.h"
#include "port/pg_numa.h"
#include "storage/pg_shmem.h"
@@ -36,8 +35,6 @@
#include <numa.h>
#include <numaif.h>
-Datum pg_numa_available(PG_FUNCTION_ARGS);
-
/* libnuma requires initialization as per numa(3) on Linux */
int
pg_numa_init(void)
@@ -66,8 +63,6 @@ pg_numa_get_max_node(void)
#else
-Datum pg_numa_available(PG_FUNCTION_ARGS);
-
/* Empty wrappers */
int
pg_numa_init(void)
@@ -89,32 +84,3 @@ pg_numa_get_max_node(void)
}
#endif
-
-Datum
-pg_numa_available(PG_FUNCTION_ARGS)
-{
- PG_RETURN_BOOL(pg_numa_init() != -1);
-}
-
-/* This should be used only after the server is started */
-Size
-pg_numa_get_pagesize(void)
-{
- Size os_page_size;
-#ifdef WIN32
- SYSTEM_INFO sysinfo;
-
- GetSystemInfo(&sysinfo);
- os_page_size = sysinfo.dwPageSize;
-#else
- os_page_size = sysconf(_SC_PAGESIZE);
-#endif
-
- Assert(IsUnderPostmaster);
- Assert(huge_pages_status != HUGE_PAGES_UNKNOWN);
-
- if (huge_pages_status == HUGE_PAGES_ON)
- GetHugePageSize(&os_page_size, NULL);
-
- return os_page_size;
-}
diff --git a/src/backend/storage/ipc/shmem.c b/src/backend/storage/ipc/shmem.c
index e10b380e5c7..0903eb50f54 100644
--- a/src/backend/storage/ipc/shmem.c
+++ b/src/backend/storage/ipc/shmem.c
@@ -93,6 +93,8 @@ static HTAB *ShmemIndex = NULL; /* primary index hashtable for shmem */
/* To get reliable results for NUMA inquiry we need to "touch pages" once */
static bool firstNumaTouch = true;
+Datum pg_numa_available(PG_FUNCTION_ARGS);
+
/*
* InitShmemAccess() --- set up basic pointers to shared memory.
*/
@@ -615,7 +617,7 @@ pg_get_shmem_allocations_numa(PG_FUNCTION_ARGS)
* This information is needed before calling move_pages() for NUMA memory
* node inquiry.
*/
- os_page_size = pg_numa_get_pagesize();
+ os_page_size = pg_get_shmem_pagesize();
/*
* Allocate memory for page pointers and status based on total shared
@@ -727,3 +729,32 @@ pg_get_shmem_allocations_numa(PG_FUNCTION_ARGS)
return (Datum) 0;
}
+
+/* This should be used only after the server is started */
+Size
+pg_get_shmem_pagesize(void)
+{
+ Size os_page_size;
+#ifdef WIN32
+ SYSTEM_INFO sysinfo;
+
+ GetSystemInfo(&sysinfo);
+ os_page_size = sysinfo.dwPageSize;
+#else
+ os_page_size = sysconf(_SC_PAGESIZE);
+#endif
+
+ Assert(IsUnderPostmaster);
+ Assert(huge_pages_status != HUGE_PAGES_UNKNOWN);
+
+ if (huge_pages_status == HUGE_PAGES_ON)
+ GetHugePageSize(&os_page_size, NULL);
+
+ return os_page_size;
+}
+
+Datum
+pg_numa_available(PG_FUNCTION_ARGS)
+{
+ PG_RETURN_BOOL(pg_numa_init() != -1);
+}
diff --git a/src/include/port/pg_numa.h b/src/include/port/pg_numa.h
index 7e990d9f776..40f1d324dcf 100644
--- a/src/include/port/pg_numa.h
+++ b/src/include/port/pg_numa.h
@@ -14,12 +14,9 @@
#ifndef PG_NUMA_H
#define PG_NUMA_H
-#include "fmgr.h"
-
extern PGDLLIMPORT int pg_numa_init(void);
extern PGDLLIMPORT int pg_numa_query_pages(int pid, unsigned long count, void **pages, int *status);
extern PGDLLIMPORT int pg_numa_get_max_node(void);
-extern PGDLLIMPORT Size pg_numa_get_pagesize(void);
#ifdef USE_LIBNUMA
diff --git a/src/include/storage/shmem.h b/src/include/storage/shmem.h
index 904a336b851..c1f668ded95 100644
--- a/src/include/storage/shmem.h
+++ b/src/include/storage/shmem.h
@@ -41,6 +41,8 @@ extern void *ShmemInitStruct(const char *name, Size size, bool *foundPtr);
extern Size add_size(Size s1, Size s2);
extern Size mul_size(Size s1, Size s2);
+extern PGDLLIMPORT Size pg_get_shmem_pagesize(void);
+
/* ipci.c */
extern void RequestAddinShmemSpace(Size size);
diff --git a/src/port/Makefile b/src/port/Makefile
index 4274949dfa4..f11896440d5 100644
--- a/src/port/Makefile
+++ b/src/port/Makefile
@@ -45,7 +45,6 @@ OBJS = \
path.o \
pg_bitutils.o \
pg_localeconv_r.o \
- pg_numa.o \
pg_popcount_aarch64.o \
pg_popcount_avx512.o \
pg_strong_random.o \
diff --git a/src/port/meson.build b/src/port/meson.build
index fc7b059fee5..48d2dfb7cf3 100644
--- a/src/port/meson.build
+++ b/src/port/meson.build
@@ -8,7 +8,6 @@ pgport_sources = [
'path.c',
'pg_bitutils.c',
'pg_localeconv_r.c',
- 'pg_numa.c',
'pg_popcount_aarch64.c',
'pg_popcount_avx512.c',
'pg_strong_random.c',
