On Tue, Jun 20, 2023 at 06:44:20PM -0500, Justin Pryzby wrote:
> On Tue, Jun 13, 2023 at 02:50:30PM +0900, Michael Paquier wrote:
>> On Mon, Jun 12, 2023 at 02:37:15PM -0700, Nathan Bossart wrote:
>> > Fair enough. I know I've been waffling in the GUC versus function
>> > discussion, but FWIW v7 of the patch looks reasonable to me.
>>
>> + Assert(strcmp("unknown", GetConfigOption("huge_pages_status", false,
>> false)) != 0);
>>
>> Not sure that there is anything to gain with this assertion in
>> CreateSharedMemoryAndSemaphores(), because this is pretty much what
>> check_GUC_init() looks after?
>
> It seems like you misread the assertion, so I added a comment about it.
> Indeed, the assertion addresses the other question you asked later.
>
> That's what I already commented about, and the reason I found it
> compelling not to use a boolean.Apologies for the late reply here. At the end, I am on board with the addition of this assertion and its position after PGSharedMemoryCreate(). I would also move the SetConfigOption() for the WIN32 path after ew have passed all the checks. There are a few FATALs that can be triggered so it would be a waste to call it if we are going to fail the shmem creation in this path. I could not resist adding two checks in the TAP tests to make sure that we don't report unknown. Perhaps that's not necessary, but that would provide coverage in a more broader way, and these are cheap. I have run one indentation, while on it. Note to self: check that manually on Windows. -- Michael
From 2ef2b73215b5775bf6d44fa9181de2cf9a379fc7 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Fri, 23 Jun 2023 13:56:04 +0900
Subject: [PATCH v9] add GUC: huge_pages_status
This is useful to show the current state of huge pages when
huge_pages=try. The effective status is not otherwise visible without
OS level tools like gdb or /proc/N/smaps. Like the other GUCs related
to huge pages, this works for Linux and Windows.
---
src/include/storage/pg_shmem.h | 5 +++--
src/backend/port/sysv_shmem.c | 14 ++++++++++++++
src/backend/port/win32_shmem.c | 5 +++++
src/backend/storage/ipc/ipci.c | 7 +++++++
src/backend/utils/misc/guc_tables.c | 19 +++++++++++++++++++
src/test/authentication/t/003_peer.pl | 6 ++++++
src/test/authentication/t/005_sspi.pl | 4 ++++
doc/src/sgml/config.sgml | 23 ++++++++++++++++++++++-
8 files changed, 80 insertions(+), 3 deletions(-)
diff --git a/src/include/storage/pg_shmem.h b/src/include/storage/pg_shmem.h
index 4dd05f156d..ba0cdc13c7 100644
--- a/src/include/storage/pg_shmem.h
+++ b/src/include/storage/pg_shmem.h
@@ -46,12 +46,13 @@ extern PGDLLIMPORT int shared_memory_type;
extern PGDLLIMPORT int huge_pages;
extern PGDLLIMPORT int huge_page_size;
-/* Possible values for huge_pages */
+/* Possible values for huge_pages and huge_pages_status */
typedef enum
{
HUGE_PAGES_OFF,
HUGE_PAGES_ON,
- HUGE_PAGES_TRY
+ HUGE_PAGES_TRY, /* only for huge_pages */
+ HUGE_PAGES_UNKNOWN /* only for huge_pages_status */
} HugePagesType;
/* Possible values for shared_memory_type */
diff --git a/src/backend/port/sysv_shmem.c b/src/backend/port/sysv_shmem.c
index eaba244bc9..f1eb5a1e20 100644
--- a/src/backend/port/sysv_shmem.c
+++ b/src/backend/port/sysv_shmem.c
@@ -627,6 +627,14 @@ CreateAnonymousSegment(Size *size)
}
#endif
+ /*
+ * Report whether huge pages are in use. This needs to be tracked before
+ * the second mmap() call if attempting to use huge pages failed
+ * previously.
+ */
+ SetConfigOption("huge_pages_status", (ptr == MAP_FAILED) ? "off" : "on",
+ PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT);
+
if (ptr == MAP_FAILED && huge_pages != HUGE_PAGES_ON)
{
/*
@@ -737,8 +745,14 @@ PGSharedMemoryCreate(Size size,
sysvsize = sizeof(PGShmemHeader);
}
else
+ {
sysvsize = size;
+ /* huge pages are only available with mmap */
+ SetConfigOption("huge_pages_status", "off",
+ PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT);
+ }
+
/*
* Loop till we find a free IPC key. Trust CreateDataDirLockFile() to
* ensure no more than one postmaster per data directory can enter this
diff --git a/src/backend/port/win32_shmem.c b/src/backend/port/win32_shmem.c
index 62e0807477..05494c14a9 100644
--- a/src/backend/port/win32_shmem.c
+++ b/src/backend/port/win32_shmem.c
@@ -401,6 +401,11 @@ retry:
on_shmem_exit(pgwin32_SharedMemoryDelete, PointerGetDatum(hmap2));
*shim = hdr;
+
+ /* Report whether huge pages are in use */
+ SetConfigOption("huge_pages_status", (flProtect & SEC_LARGE_PAGES) ?
+ "on" : "off", PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT);
+
return hdr;
}
diff --git a/src/backend/storage/ipc/ipci.c b/src/backend/storage/ipc/ipci.c
index 8f1ded7338..cc387c00a1 100644
--- a/src/backend/storage/ipc/ipci.c
+++ b/src/backend/storage/ipc/ipci.c
@@ -190,6 +190,13 @@ CreateSharedMemoryAndSemaphores(void)
*/
seghdr = PGSharedMemoryCreate(size, &shim);
+ /*
+ * Make sure that huge pages are never reported as "unknown" while the
+ * server is running.
+ */
+ Assert(strcmp("unknown",
+ GetConfigOption("huge_pages_status", false, false)) != 0);
+
InitShmemAccess(seghdr);
/*
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 71e27f8eb0..189b407b1a 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -365,6 +365,13 @@ static const struct config_enum_entry huge_pages_options[] = {
{NULL, 0, false}
};
+static const struct config_enum_entry huge_pages_status_options[] = {
+ {"off", HUGE_PAGES_OFF, false},
+ {"on", HUGE_PAGES_ON, false},
+ {"unknown", HUGE_PAGES_UNKNOWN, false},
+ {NULL, 0, false}
+};
+
static const struct config_enum_entry recovery_prefetch_options[] = {
{"off", RECOVERY_PREFETCH_OFF, false},
{"on", RECOVERY_PREFETCH_ON, false},
@@ -550,6 +557,7 @@ int ssl_renegotiation_limit;
*/
int huge_pages = HUGE_PAGES_TRY;
int huge_page_size;
+int huge_pages_status = HUGE_PAGES_UNKNOWN;
/*
* These variables are all dummies that don't do anything, except in some
@@ -4876,6 +4884,17 @@ struct config_enum ConfigureNamesEnum[] =
NULL, NULL, NULL
},
+ {
+ {"huge_pages_status", PGC_INTERNAL, PRESET_OPTIONS,
+ gettext_noop("Indicates the status of huge pages."),
+ NULL,
+ GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE
+ },
+ &huge_pages_status,
+ HUGE_PAGES_UNKNOWN, huge_pages_status_options,
+ NULL, NULL, NULL
+ },
+
{
{"recovery_prefetch", PGC_SIGHUP, WAL_RECOVERY,
gettext_noop("Prefetch referenced blocks during recovery."),
diff --git a/src/test/authentication/t/003_peer.pl b/src/test/authentication/t/003_peer.pl
index d8e4976072..eacff2b52a 100644
--- a/src/test/authentication/t/003_peer.pl
+++ b/src/test/authentication/t/003_peer.pl
@@ -100,6 +100,12 @@ my $system_user =
$node->safe_psql('postgres',
q(select (string_to_array(SYSTEM_USER, ':'))[2]));
+# While on it, check the status of huge pages, that can be either on
+# or off, but never unknown.
+my $huge_pages_status =
+ $node->safe_psql('postgres', q(SHOW huge_pages_status;));
+isnt($huge_pages_status, 'unknown', "check huge_pages_status");
+
# Tests without the user name map.
# Failure as connection is attempted with a database role not mapping
# to an authorized system user.
diff --git a/src/test/authentication/t/005_sspi.pl b/src/test/authentication/t/005_sspi.pl
index 05d81f3422..37fd5bc243 100644
--- a/src/test/authentication/t/005_sspi.pl
+++ b/src/test/authentication/t/005_sspi.pl
@@ -21,6 +21,10 @@ $node->init;
$node->append_conf('postgresql.conf', "log_connections = on\n");
$node->start;
+my $huge_pages_status =
+ $node->safe_psql('postgres', q(SHOW huge_pages_status;));
+isnt($huge_pages_status, 'unknown', "check huge_pages_status");
+
# SSPI is set up by default. Make sure it interacts correctly with
# require_auth.
$node->connect_ok("require_auth=sspi",
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 6262cb7bb2..c9fa6cd9c7 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1727,7 +1727,9 @@ include_dir 'conf.d'
server will try to request huge pages, but fall back to the default if
that fails. With <literal>on</literal>, failure to request huge pages
will prevent the server from starting up. With <literal>off</literal>,
- huge pages will not be requested.
+ huge pages will not be requested. The actual state of huge pages is
+ indicated by the server variable
+ <xref linkend="guc-huge-pages-status"/>.
</para>
<para>
@@ -10738,6 +10740,25 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
</listitem>
</varlistentry>
+ <varlistentry id="guc-huge-pages-status" xreflabel="huge_pages_status">
+ <term><varname>huge_pages_status</varname> (<type>enum</type>)
+ <indexterm>
+ <primary><varname>huge_pages_status</varname> configuration parameter</primary>
+ </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Reports the state of huge pages in the current instance:
+ <literal>on</literal>, <literal>off</literal>, or
+ <literal>unknown</literal> (if displayed with
+ <literal>postgres -C</literal>).
+ This parameter is useful to determine whether allocation of huge pages
+ was successful under <literal>huge_pages=try</literal>.
+ See <xref linkend="guc-huge-pages"/> for more information.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="guc-integer-datetimes" xreflabel="integer_datetimes">
<term><varname>integer_datetimes</varname> (<type>boolean</type>)
<indexterm>
--
2.40.1
signature.asc
Description: PGP signature
