On Fri, Jan 23, 2026 at 4:53 AM Michael Paquier <[email protected]> wrote:
>
> On Thu, Jan 22, 2026 at 02:17:08PM -0500, Tom Lane wrote:
> > Ashutosh Bapat <[email protected]> writes:
> >> Over [1] Peter mentioned that PG_MMAP_FLAGS is not used for
> >> portability even though it's placed in portability/mem.h. That might
> >> have been the intention when it was added in
> >> b0fc0df9364d2d2d17c0162cf3b8b59f6cb09f67. But history does not show it
> >> being used that way at any point in time. Per suggestion removing that
> >> macro and instead using the flags directly in CreateAnonymousSegment()
> >> which is the only place where it's used.
> >
> > I think you attached the wrong patch?  This one doesn't touch
> > PG_MMAP_FLAGS.

I didn't do a good job writing that email: attached wrong patch and
also didn't provide the required reference. Sorry for that. Corrected
in this email.

>
> PG_MMAP_FLAGS is still used in two places in sysv_shmem.c, where I
> guess the intention of Robert back in b0fc0df9364d was to not
> copy-paste the same flag values multiple times.  I can still get the
> intention even today, so, if we were to do something, why don't you
> just make PG_MMAP_FLAGS local to sysv_shmem.c and call it a day?
>
> Honestly, I don't think that we should change this code at all: I also
> like the current idea of PG_MMAP_FLAGS being defined in the same place
> where we check for HASSEMAPHORE and ANONYMOUS, so it comes down to
> this being a matter of taste.

As Peter mentioned in the shared buffers resizing thread [1] it's
placement and name in mem.h led us to think that it affects all mmap
calls or that all mmap calls should use those flags. Neither of which
is true. We have different mmap calls using different set of flags.
Attached patch proposes changes similar (not exact) as you suggest
above. Please take a look.

[1] 
https://www.postgresql.org/message-id/[email protected]

-- 
Best Wishes,
Ashutosh Bapat
From 7e8a4457c8640fd62cf562e98c0b0df9ae80157f Mon Sep 17 00:00:00 2001
From: Ashutosh Bapat <[email protected]>
Date: Thu, 22 Jan 2026 20:48:36 +0530
Subject: [PATCH v20260123] Remove PG_MMAP_FLAGS

Though the macro seems to be something meant to be used for all mmap calls, it
is actually used only in CreateAnonymousSegment(). Other calls to mmap() use
different set of flags. The macros was introduced in commit
b0fc0df9364d2d2d17c0162cf3b8b59f6cb09f67, but was never used for portability.
Remove it and use the flags directly in CreateAnonymousSegment().

Author: Ashutosh Bapat <[email protected]>
Suggested by: Peter Eisentraut <[email protected]>
Discussion: https://www.postgresql.org/message-id/[email protected]
---
 src/backend/port/sysv_shmem.c | 10 ++++++----
 src/include/portability/mem.h |  2 --
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/backend/port/sysv_shmem.c b/src/backend/port/sysv_shmem.c
index de491897118..0f4ecac16e8 100644
--- a/src/backend/port/sysv_shmem.c
+++ b/src/backend/port/sysv_shmem.c
@@ -601,6 +601,8 @@ CreateAnonymousSegment(Size *size)
 	Size		allocsize = *size;
 	void	   *ptr = MAP_FAILED;
 	int			mmap_errno = 0;
+	int			mmap_flags = (MAP_SHARED | MAP_ANONYMOUS | MAP_HASSEMAPHORE);
+
 
 #ifndef MAP_HUGETLB
 	/* PGSharedMemoryCreate should have dealt with this case */
@@ -612,15 +614,15 @@ CreateAnonymousSegment(Size *size)
 		 * Round up the request size to a suitable large value.
 		 */
 		Size		hugepagesize;
-		int			mmap_flags;
+		int			huge_mmap_flags;
 
-		GetHugePageSize(&hugepagesize, &mmap_flags);
+		GetHugePageSize(&hugepagesize, &huge_mmap_flags);
 
 		if (allocsize % hugepagesize != 0)
 			allocsize += hugepagesize - (allocsize % hugepagesize);
 
 		ptr = mmap(NULL, allocsize, PROT_READ | PROT_WRITE,
-				   PG_MMAP_FLAGS | mmap_flags, -1, 0);
+				   mmap_flags | huge_mmap_flags, -1, 0);
 		mmap_errno = errno;
 		if (huge_pages == HUGE_PAGES_TRY && ptr == MAP_FAILED)
 			elog(DEBUG1, "mmap(%zu) with MAP_HUGETLB failed, huge pages disabled: %m",
@@ -644,7 +646,7 @@ CreateAnonymousSegment(Size *size)
 		 */
 		allocsize = *size;
 		ptr = mmap(NULL, allocsize, PROT_READ | PROT_WRITE,
-				   PG_MMAP_FLAGS, -1, 0);
+				   mmap_flags, -1, 0);
 		mmap_errno = errno;
 	}
 
diff --git a/src/include/portability/mem.h b/src/include/portability/mem.h
index 091328f680d..c048e8836c5 100644
--- a/src/include/portability/mem.h
+++ b/src/include/portability/mem.h
@@ -38,8 +38,6 @@
 #define MAP_NOSYNC			0
 #endif
 
-#define PG_MMAP_FLAGS			(MAP_SHARED|MAP_ANONYMOUS|MAP_HASSEMAPHORE)
-
 /* Some really old systems don't define MAP_FAILED. */
 #ifndef MAP_FAILED
 #define MAP_FAILED ((void *) -1)

base-commit: f9a468c664a4605e3080383ffaa302057d56feb1
-- 
2.34.1

Reply via email to