On Thu, Mar 23, 2023 at 05:25:46PM +0900, Kyotaro Horiguchi wrote: > Wouldn't storing the value in the shared memory itself work? Though, > that space does become almost dead for the server's lifetime...
I'm sure it's possible, but it's also not worth writing a special implementation just to handle huge_pages_active, which is better written in 30 lines than in 300 lines. If we needed to avoid using a GUC, maybe it'd work to add huge_pages_active to PGShmemHeader. But "avoid using gucs at all costs" isn't the goal, and using a GUC parallels all the similar, and related things without needing to allocate extra bits of shared_memory. On Thu, Mar 23, 2023 at 07:23:28AM +0900, Michael Paquier wrote: > On Wed, Mar 22, 2023 at 05:18:28PM -0500, Justin Pryzby wrote: > > Wow, good point. I think to make it work we'd need put > > huge_pages_active into BackendParameters and handle it in > > save_backend_variables(). If so, that'd be strong argument for using a > > GUC, which already has all the necessary infrastructure for exposing the > > server's state. > > I am afraid so, duplicating an existing infrastructure for a need like > the one of this thread is not really appealing. This goes back to using a GUC, and: - removes GUC_RUNTIME_COMPUTED, since that causes a useless error when using -C if the server is running, rather than successfully printing "unknown". - I also renamed it from huge_pages_active = {true,false,unknown} to huge_pages_STATUS = {on,off,unknown}. This parallels huge_pages, which is documented to accept values on/off/try. Also, true/false isn't how bools are displayed. - I realized that the rename suggested implementing it as an enum GUC, and re-using the existing HUGE_PAGES_{ON,OFF} values (and adding an "UNKNOWN"). Maybe this also avoids Stephen's earlier objection to using a string ? I mis-used cirrusci to check that the GUC does work correctly under windows. If there's continuing aversions to using a GUC, please say so, and try to suggest an alternate implementation you think would be justified. It'd need to work under windows with EXEC_BACKEND. -- Justin
>From 951d481b74ac978482b9d5794b437f741518f3a9 Mon Sep 17 00:00:00 2001 From: Justin Pryzby <pryz...@telsasoft.com> Date: Mon, 23 Jan 2023 18:33:51 -0600 Subject: [PATCH v6] 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/tu4pr8401mb1152ebb0d271f827e2e37a01ee...@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 | 20 ++++++++++++++++++++ src/include/storage/pg_shmem.h | 5 +++-- 5 files changed, 46 insertions(+), 3 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 510ff88fc5e..88998238645 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-status"/>. </para> <para> @@ -10660,6 +10661,20 @@ 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. + 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..b6104c6ce60 100644 --- a/src/backend/port/sysv_shmem.c +++ b/src/backend/port/sysv_shmem.c @@ -627,6 +627,9 @@ CreateAnonymousSegment(Size *size) } #endif + SetConfigOption("huge_pages_status", ptr == MAP_FAILED ? "off" : "on", + PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT); + if (ptr == MAP_FAILED && huge_pages != HUGE_PAGES_ON) { /* diff --git a/src/backend/port/win32_shmem.c b/src/backend/port/win32_shmem.c index 62e08074770..199f2e23a13 100644 --- a/src/backend/port/win32_shmem.c +++ b/src/backend/port/win32_shmem.c @@ -327,6 +327,10 @@ retry: Sleep(1000); continue; } + + SetConfigOption("huge_pages_status", (flProtect & SEC_LARGE_PAGES) ? + "on" : "off", PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT); + break; } diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index 8696d965dee..785bee948e9 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -348,6 +348,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}, @@ -536,6 +543,8 @@ 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 * cases provide the value for SHOW to display. The real state is elsewhere @@ -4833,6 +4842,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/include/storage/pg_shmem.h b/src/include/storage/pg_shmem.h index 4dd05f156d5..bfda42b0b26 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/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 */ -- 2.34.1