Date: Mon, 13 Feb 2023 17:22:45 -0600
From: Justin Pryzby <pryzby@telsasoft.com>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>,
	Andres Freund <andres@anarazel.de>,
	Michael Paquier <michael@paquier.xyz>,
	Kyotaro Horiguchi <horikyota.ntt@gmail.com>, thomas.munro@gmail.com,
	tgl@sss.pgh.pa.us, john.naylor@enterprisedb.com,
	noriyoshi.shinoda@hpe.com, jchampion@timescale.com,
	sawada.mshk@gmail.com, masao.fujii@oss.nttdata.com,
	pgsql-hackers@postgresql.org, rjuju123@gmail.com
Subject: Re: Improve logging when using Huge Pages
Message-ID: <20230213232245.GI1653@telsasoft.com>
References: <20230208231803.GP1653@telsasoft.com>
 <20230209094013.67varf63bihops33@alvherre.pgsql>
 <20230209192909.GA606606@nathanxps13>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="uWbmMdDzzl2TXAgx"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20230209192909.GA606606@nathanxps13>
User-Agent: Mutt/1.9.4 (2018-02-28)
Archived-At: <https://www.postgresql.org/message-id/20230213232245.GI1653%40telsasoft.com>
Precedence: bulk


--uWbmMdDzzl2TXAgx
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

On Thu, Feb 09, 2023 at 11:29:09AM -0800, Nathan Bossart wrote:
> On Thu, Feb 09, 2023 at 10:40:13AM +0100, Alvaro Herrera wrote:
> > On 2023-Feb-08, Justin Pryzby wrote:
> >> I don't think it makes sense to run postgres -C huge_pages_active,
> >> however, so I see no issue that that would always returns "false".
> > 
> > Hmm, I would initialize it to return "unknown" rather than "off" — and
> > make sure it turns "off" at the appropriate time.  Otherwise you're just
> > moving the confusion elsewhere.
> 
> I think this approach would address my concerns about using a GUC.

Done that way.  This also fixes the logic in win32_shmem.c.

-- 
Justin

--uWbmMdDzzl2TXAgx
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="v3-0001-add-GUC-huge_pages_active.patch"

From 5ead7ba3711dd7a0bb9ec083ebd60049f50f9907 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Mon, 23 Jan 2023 18:33:51 -0600
Subject: [PATCH v3] add GUC: huge_pages_active

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.

https://www.postgresql.org/message-id/flat/TU4PR8401MB1152EBB0D271F827E2E37A01EECC9@TU4PR8401MB1152.NAMPRD84.PROD.OUTLOOK.COM
---
 doc/src/sgml/config.sgml            | 17 ++++++++++++++++-
 src/backend/port/sysv_shmem.c       |  3 +++
 src/backend/port/win32_shmem.c      |  4 ++++
 src/backend/utils/misc/guc_tables.c | 12 ++++++++++++
 4 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 8c56b134a84..3770c7dc254 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1700,23 +1700,24 @@ include_dir 'conf.d'
       </indexterm>
       </term>
       <listitem>
        <para>
         Controls whether huge pages are requested for the main shared memory
         area. Valid values are <literal>try</literal> (the default),
         <literal>on</literal>, and <literal>off</literal>.  With
         <varname>huge_pages</varname> set to <literal>try</literal>, the
         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-active"/>.
        </para>
 
        <para>
         At present, this setting is supported only on Linux and Windows. The
         setting is ignored on other systems when set to
         <literal>try</literal>.  On Linux, it is only supported when
         <varname>shared_memory_type</varname> is set to <literal>mmap</literal>
         (the default).
        </para>
 
        <para>
@@ -10679,22 +10680,36 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
         with assertions enabled. That is the case if the
         macro <symbol>USE_ASSERT_CHECKING</symbol> is defined
         when <productname>PostgreSQL</productname> is built (accomplished
         e.g., by the <command>configure</command> option
         <option>--enable-cassert</option>). By
         default <productname>PostgreSQL</productname> is built without
         assertions.
        </para>
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-huge-pages-active" xreflabel="huge_pages_active">
+      <term><varname>huge_pages_active</varname> (<type>boolean</type>)
+      <indexterm>
+       <primary><varname>huge_pages_active</varname> configuration parameter</primary>
+      </indexterm>
+      </term>
+      <listitem>
+       <para>
+        Reports whether huge pages are in use by the current process.
+        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>
        <primary><varname>integer_datetimes</varname> configuration parameter</primary>
       </indexterm>
       </term>
       <listitem>
        <para>
         Reports whether <productname>PostgreSQL</productname> was built with support for
         64-bit-integer dates and times.  As of <productname>PostgreSQL</productname> 10,
         this is always <literal>on</literal>.
diff --git a/src/backend/port/sysv_shmem.c b/src/backend/port/sysv_shmem.c
index eaba244bc9c..48ead4d27bf 100644
--- a/src/backend/port/sysv_shmem.c
+++ b/src/backend/port/sysv_shmem.c
@@ -619,22 +619,25 @@ CreateAnonymousSegment(Size *size)
 			allocsize += hugepagesize - (allocsize % hugepagesize);
 
 		ptr = mmap(NULL, allocsize, PROT_READ | PROT_WRITE,
 				   PG_MMAP_FLAGS | 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",
 				 allocsize);
 	}
 #endif
 
+	SetConfigOption("huge_pages_active", ptr == MAP_FAILED ? "off" : "on",
+					PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT);
+
 	if (ptr == MAP_FAILED && huge_pages != HUGE_PAGES_ON)
 	{
 		/*
 		 * Use the original size, not the rounded-up value, when falling back
 		 * to non-huge pages.
 		 */
 		allocsize = *size;
 		ptr = mmap(NULL, allocsize, PROT_READ | PROT_WRITE,
 				   PG_MMAP_FLAGS, -1, 0);
 		mmap_errno = errno;
 	}
diff --git a/src/backend/port/win32_shmem.c b/src/backend/port/win32_shmem.c
index 62e08074770..0bf594c8bf9 100644
--- a/src/backend/port/win32_shmem.c
+++ b/src/backend/port/win32_shmem.c
@@ -319,22 +319,26 @@ retry:
 		 * If the segment already existed, CreateFileMapping() will return a
 		 * handle to the existing one and set ERROR_ALREADY_EXISTS.
 		 */
 		if (GetLastError() == ERROR_ALREADY_EXISTS)
 		{
 			CloseHandle(hmap);	/* Close the handle, since we got a valid one
 								 * to the previous segment. */
 			hmap = NULL;
 			Sleep(1000);
 			continue;
 		}
+
+		SetConfigOption("huge_pages_active", (flProtect & SEC_LARGE_PAGES) ?
+						"on" : "off", PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT);
+
 		break;
 	}
 
 	/*
 	 * If the last call in the loop still returned ERROR_ALREADY_EXISTS, this
 	 * shared memory segment exists and we assume it belongs to somebody else.
 	 */
 	if (!hmap)
 		ereport(FATAL,
 				(errmsg("pre-existing shared memory block is still in use"),
 				 errhint("Check if there are any old server processes still running, and terminate them.")));
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index b21698934c8..1a298f16c81 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -554,22 +554,23 @@ static int	server_version_num;
 #define	DEFAULT_SYSLOG_FACILITY LOG_LOCAL0
 #else
 #define	DEFAULT_SYSLOG_FACILITY 0
 #endif
 static int	syslog_facility = DEFAULT_SYSLOG_FACILITY;
 
 static char *timezone_string;
 static char *log_timezone_string;
 static char *timezone_abbreviations_string;
 static char *data_directory;
 static char *session_authorization_string;
+static char *huge_pages_active = "unknown"; /* dynamically set */
 static int	max_function_args;
 static int	max_index_keys;
 static int	max_identifier_length;
 static int	block_size;
 static int	segment_size;
 static int	shared_memory_size_mb;
 static int	shared_memory_size_in_huge_pages;
 static int	wal_block_size;
 static bool data_checksums;
 static bool integer_datetimes;
 
@@ -4516,22 +4517,33 @@ struct config_string ConfigureNamesString[] =
 	{
 		{"backtrace_functions", PGC_SUSET, DEVELOPER_OPTIONS,
 			gettext_noop("Log backtrace for errors in these functions."),
 			NULL,
 			GUC_NOT_IN_SAMPLE
 		},
 		&backtrace_functions,
 		"",
 		check_backtrace_functions, assign_backtrace_functions, NULL
 	},
 
+	{
+		{"huge_pages_active", PGC_INTERNAL, PRESET_OPTIONS,
+			gettext_noop("Indicates whether huge pages are in use."),
+			NULL,
+			GUC_NOT_IN_SAMPLE | GUC_DISALLOW_IN_FILE | GUC_RUNTIME_COMPUTED
+		},
+		&huge_pages_active,
+		"unknown",
+		NULL, NULL, NULL
+	},
+
 	/* End-of-list marker */
 	{
 		{NULL, 0, 0, NULL, NULL}, NULL, NULL, NULL, NULL, NULL
 	}
 };
 
 
 struct config_enum ConfigureNamesEnum[] =
 {
 	{
 		{"backslash_quote", PGC_USERSET, COMPAT_OPTIONS_PREVIOUS,
-- 
2.34.1


--uWbmMdDzzl2TXAgx--