On Wed, Apr 9, 2025 at 12:48 AM Tomas Vondra <to...@vondra.me> wrote: > 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
Hi Tomas, sorry for this. I've tested the numa-fixes.patch, CI says it is ok to me (but it was always like that), meson test also looks good here, and some other quick use attempts are OK. I can confirm that `ninja -t missingdeps` is clean with this (without it really wasn't; +1 to adding this check to CI). > > 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. True, the pg_shm_get_page_size() looks like a great middleground (not in numa but still for shm w/ hugepages). > > > > 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. -J.
