From 2df0a06b206dedfebd6f4ef7f00eed15edbdee53 Mon Sep 17 00:00:00 2001 From: Tomas Vondra Date: Thu, 3 Apr 2025 12:43:21 +0200 Subject: [PATCH v22 5/7] review --- contrib/pg_buffercache/pg_buffercache_pages.c | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/contrib/pg_buffercache/pg_buffercache_pages.c b/contrib/pg_buffercache/pg_buffercache_pages.c index 3460cf579f7..dc200204478 100644 --- a/contrib/pg_buffercache/pg_buffercache_pages.c +++ b/contrib/pg_buffercache/pg_buffercache_pages.c @@ -90,7 +90,7 @@ pg_buffercache_numa_prepare_ptrs(int buffer_id, double pages_per_blk, Size os_page_size, void **os_page_ptrs) { - + /* ??? */ /* XXX: move it here? */ } #endif @@ -343,14 +343,28 @@ pg_buffercache_numa_pages(PG_FUNCTION_ARGS) buffers_per_page = os_page_size / BLCKSZ; pages_per_buffer = BLCKSZ / os_page_size; + /* + * The pages and block size is expected to be 2^k, so one divides the + * other (we don't know in which direction). + */ + Assert((os_page_size % BLCKSZ == 0) || (BLCKSZ % os_page_size == 0)); + + /* + * Either both counts are 1 (when the pages have the same size), or + * exacly one of them is zero. Both can't be zero at the same time. + */ + Assert((buffers_per_page > 0) || (pages_per_buffer > 0)); + Assert(((buffers_per_page == 1) && (pages_per_buffer == 1)) || + ((buffers_per_page == 0) || (pages_per_buffer == 0))); + /* * How many addresses we are going to query (store) depends on the - * relation between BLCKSZ : PAGESIZE. + * relation between BLCKSZ : PAGESIZE. We need at least one status + * per buffer - if the memory page is larger than buffer, we still + * query it for each buffer. With multiple memory pages per buffer, + * we need that many entries. */ - if (buffers_per_page > 1) - os_page_query_count = NBuffers; - else - os_page_query_count = NBuffers * pages_per_buffer; + os_page_query_count = Max(NBuffers, NBuffers * pages_per_buffer); elog(DEBUG1, "NUMA: NBuffers=%d os_page_query_count=" UINT64_FORMAT " os_page_size=%zu buffers_per_page=%d pages_per_buffer=%d", NBuffers, os_page_query_count, os_page_size, buffers_per_page, pages_per_buffer); @@ -361,7 +375,6 @@ pg_buffercache_numa_pages(PG_FUNCTION_ARGS) /* * If we ever get 0xff back from kernel inquiry, then we probably have * bug in our buffers to OS page mapping code here. - * */ memset(os_pages_status, 0xff, sizeof(int) * os_page_query_count); @@ -410,8 +423,8 @@ pg_buffercache_numa_pages(PG_FUNCTION_ARGS) /* * Altough we could query just once per each OS page, we do it * repeatably for each Buffer and hit the same address as - * move_pages(2) requires page aligment. This is also - * simplifies retrieval code later on. + * move_pages(2) requires page aligment. This also simplifies + * retrieval code later on. */ os_page_ptrs[i] = (char *) TYPEALIGN(os_page_size, (char *) BufferGetBlock(i + 1)); -- 2.49.0