Date: Wed, 8 Feb 2023 17:18:03 -0600
From: Justin Pryzby <pryzby@telsasoft.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: 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,
	Nathan Bossart <nathandbossart@gmail.com>
Subject: Re: Improve logging when using Huge Pages
Message-ID: <20230208231803.GP1653@telsasoft.com>
References: <20230125013728.GK13860@telsasoft.com>
 <20230202155337.gwsjcszhxhdgqmhs@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="LQAwcd5tHl0Qlnzi"
Content-Disposition: inline
In-Reply-To: <20230202155337.gwsjcszhxhdgqmhs@alvherre.pgsql>
User-Agent: Mutt/1.9.4 (2018-02-28)
Archived-At: 
 <https://www.postgresql.org/message-id/20230208231803.GP1653%40telsasoft.com>
Precedence: bulk


--LQAwcd5tHl0Qlnzi
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Thu, Feb 02, 2023 at 04:53:37PM +0100, Alvaro Herrera wrote:
> On 2023-Jan-24, Justin Pryzby wrote:
> > On Mon, Jan 23, 2023 at 05:33:35PM -0800, Andres Freund wrote:
> > > I'm ok with this being a GUC, it doesn't feel elegant, but I suspect there's
> > > no realistic elegant answer.
> 
> > Whether it's a GUC or a function, I propose to name it hugepages_active.
> > If there's no objections, I'll add to the CF.
> 
> Maybe I misread the code (actually I only read the patch), but -- does
> it set active=true when huge_pages=on?  I think the code only works when
> huge_pages=try.  That might be pretty confusing; I think it should say
> "on" in both cases.

Yes - I realized that too.   There's no reason this GUC should be
inaccurate when huge_pages=on.  (I had hoped there would be a conflict
needing resolution before anyone else noticed.)

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".
If need be, maybe the documentation could say "indicates whether huge
pages are active for the running server".

Does anybody else want to vote for a function rather than a
RUNTIME_COMPUTED GUC ?

-- 
Justin

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

From 217b01b7eddabb5d863d50b1d4ca8e1019d9413c Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Mon, 23 Jan 2023 18:33:51 -0600
Subject: [PATCH v2] 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.
---
 doc/src/sgml/config.sgml            | 17 ++++++++++++++++-
 src/backend/port/sysv_shmem.c       |  5 ++++-
 src/backend/port/win32_shmem.c      |  5 ++++-
 src/backend/utils/misc/guc_tables.c | 13 +++++++++++++
 4 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index d190be1925d..fcef4e3ce8e 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1708,7 +1708,8 @@ 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-active"/>.
        </para>
 
        <para>
@@ -10687,6 +10688,20 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
       </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>
diff --git a/src/backend/port/sysv_shmem.c b/src/backend/port/sysv_shmem.c
index eaba244bc9c..9b9b25d53ba 100644
--- a/src/backend/port/sysv_shmem.c
+++ b/src/backend/port/sysv_shmem.c
@@ -621,7 +621,10 @@ CreateAnonymousSegment(Size *size)
 		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)
+		if (ptr != MAP_FAILED)
+			SetConfigOption("huge_pages_active", "on",
+					PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT);
+		else if (huge_pages == HUGE_PAGES_TRY)
 			elog(DEBUG1, "mmap(%zu) with MAP_HUGETLB failed, huge pages disabled: %m",
 				 allocsize);
 	}
diff --git a/src/backend/port/win32_shmem.c b/src/backend/port/win32_shmem.c
index 62e08074770..73472a18f25 100644
--- a/src/backend/port/win32_shmem.c
+++ b/src/backend/port/win32_shmem.c
@@ -290,7 +290,10 @@ retry:
 								 size_low,	/* Size Lower 32 bits */
 								 szShareMem);
 
-		if (!hmap)
+		if (hmap)
+			SetConfigOption("huge_pages_active", "on",
+					PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT);
+		else
 		{
 			if (GetLastError() == ERROR_NO_SYSTEM_RESOURCES &&
 				huge_pages == HUGE_PAGES_TRY &&
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index b21698934c8..1e8e67a4c54 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -571,6 +571,7 @@ 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 huge_pages_active = false; /* dynamically set */
 static bool integer_datetimes;
 
 #ifdef USE_ASSERT_CHECKING
@@ -1013,6 +1014,18 @@ struct config_bool ConfigureNamesBool[] =
 		true,
 		NULL, NULL, 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,
+		false,
+		NULL, NULL, NULL
+	},
+
 	{
 		/* Not for general use --- used by SET SESSION AUTHORIZATION */
 		{"is_superuser", PGC_INTERNAL, UNGROUPED,
-- 
2.25.1


--LQAwcd5tHl0Qlnzi--