v22-0005-review.patch
text/x-patch
Filename: v22-0005-review.patch
Type: text/x-patch
Part: 4
Patch
Same data as JSON:
GET /api/v1/attachments/:id/patch
the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes.
API reference →
Format: format-patch
Series: patch v22-0005
Subject: review
| File | + | − |
|---|---|---|
| contrib/pg_buffercache/pg_buffercache_pages.c | 22 | 9 |
From 2df0a06b206dedfebd6f4ef7f00eed15edbdee53 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas@vondra.me>
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