Subject: BUG #17448: In Windows 10, version 1703 and later, huge_pages doesn't work.

From: PG Bug reporting form <[email-masked]>
Date: Fri, 25 Mar 2022 07:52:57 +0000

The following bug has been logged on the website:

Bug reference:      17448
Logged by:          Okano Naoki
Email address:      [email-masked]
PostgreSQL version: 14.2
Operating system:   Windows
Description:        

With huge_pages = on, the postgres process does not appear to use large
pages.
I checked with VMMap if the large pages are used in the following
environment.
Environment
  PostgreSQL version: 14.2
  Operating system  : Windows 10 20H2

On this page (*) says that in Windows 10, version 1703 and later OS
versions,  
you must specify the FILE_MAP_LARGE_PAGES flag with the MapViewOfFile
function 
to map large pages.

I think it seems to be the cause that MapViewOfFile() in
src/backend/port/win32_shmem.c 
does not specify FILE_MAP_LARGE_PAGES flag.

(*) MapViewOfFileEx function
https://docs.microsoft.com/en-us/windows/win32/api/memoryapi/nf-memoryapi-mapviewoffileex

FILE_MAP_LARGE_PAGES
  Starting with Windows 10, version 1703, this flag specifies 
  that the view should be mapped using large page support. 
  The size of the view must be a multiple of the size of a large page 
  reported by the GetLargePageMinimum function, 
  and the file-mapping object must have been created using the
SEC_LARGE_PAGES option. 
  If you provide a non-null value for lpBaseAddress, 
  then the value must be a multiple of GetLargePageMinimum.

---

From: Michael Paquier <[email-masked]>
Date: Sat, 26 Mar 2022 14:46:45 +0900

> does not specify FILE_MAP_LARGE_PAGES flag.

Hmm.  Okay.  A patch would be straight-forward, as we could just
assign the optional flag in a separate variable at the beginning of
PGSharedMemoryCreate(), similarly to flProtect when we find out that
large pages can be used, then pass it down to MapViewOfFileEx().  I
don't have a Windows 10 machine as recent as that at hand, though..

Perhaps the CI uses Windows machines that would allow to test and
check that, with some logs magically added to debug things.
--
Michael

---

From: Julien Rouhaud <[email-masked]>
Date: Sat, 26 Mar 2022 16:24:08 +0800

On Sat, Mar 26, 2022 at 02:46:45PM +0900, Michael Paquier wrote:
> I don't have a Windows 10 machine as recent as that at hand, though..

I have a Windows 10 apparently version 20H2 (the versioning doesn't make any
sense) with all needed to compile postgres at hand.  I can have a look next
week.

---

From: Michael Paquier <[email-masked]>
Date: Sat, 26 Mar 2022 18:03:55 +0900

> sense) with all needed to compile postgres at hand.  I can have a look next

Thanks!
--
Michael

---

From: Thomas Munro <[email-masked]>
Date: Sun, 27 Mar 2022 00:07:57 +1300

> sense) with all needed to compile postgres at hand.  I can have a look next

There are traces of method to the madness:  It's basically YYMM, but
then after 2004 they switched to H1 and H2 (first/second half of the
year) instead of MM, perhaps to avoid confusion with YYYY format year.
Note also that Windows 10 has a 21H2 and Windows 11 has a 21H2.

Hmm, so all versions of Windows that our current coding worked on were
EOL'd 6 months after PostgreSQL 11 came out with huge_pages support
for Windows:

https://en.wikipedia.org/wiki/Windows_10_version_history

Some question I have: is FILE_MAP_LARGE PAGES a macro?  We claim to
support all those ancient zombie OSes like Windows 7, or maybe it's
even XP for 11, and this has to be back-patched to 11, so we might
need to make it conditional.  But conditional on what?  For example,
does something like the attached work (untested)?  What happens if a <
1703 kernel sees this flag, does it reject it or ignore it?

---

From: Michael Paquier <[email-masked]>
Date: Sat, 26 Mar 2022 20:33:48 +0900

> 1703 kernel sees this flag, does it reject it or ignore it?

Good question.  I would choose a soft approach here and not insist if
the flag was not known at compilation time, but we could also take a
more aggressive approach and hardcode a value.  Anyway, it seems to me
that the correct solution here would be to compile the code with a
PG_FILE_MAP_LARGE_PAGES that checks if the flag exists at compile
time, and we would set it at run time if we know that we are on a
version of Windows that supports it.  IsWindowsVersionOrGreater()
should be able to do the job.
--
Michael

---

From: Michael Paquier <[email-masked]>
Date: Wed, 30 Mar 2022 16:54:50 +0900

> 1703 kernel sees this flag, does it reject it or ignore it?

I don't have an answer about how much Windows gets angry if we pass
down to MapViewOfFileEx() the flag FILE_MAP_LARGE_PAGES when running
the code on a version of Windows that does not support it.

Anyway, I think that we could just play it safe.  See for example the
attached, where I use PG_FILE_MAP_LARGE_PAGES at compile time to find
if the value is set.  Then, at run-time, I am just relying on 
IsWindowsVersionOrGreater() to do the job, something useful when
huge_pages=on as I guess we should fail hard if we did not know about
FILE_MAP_LARGE_PAGES at compile-time, but try to use huge pages at run
time with version >= 10.0.1703.

Perhaps there is a better thing to do?
--
Michael
diff --git a/src/backend/port/win32_shmem.c b/src/backend/port/win32_shmem.c
index 6cf69411db..49b9fa0322 100644
--- a/src/backend/port/win32_shmem.c
+++ b/src/backend/port/win32_shmem.c
@@ -12,6 +12,8 @@
  */
 #include "postgres.h"
 
+#include <versionhelpers.h>
+
 #include "miscadmin.h"
 #include "storage/dsm.h"
 #include "storage/ipc.h"
@@ -39,6 +41,17 @@
 #define PROTECTIVE_REGION_SIZE (10 * WIN32_STACK_RLIMIT)
 void	   *ShmemProtectiveRegion = NULL;
 
+/*
+ * FILE_MAP_LARGE_PAGES is needed for the support of huge pages, and
+ * it is only available since Windows 10.0.1703.  PG_FILE_MAP_LARGE_PAGES
+ * saves its value at compile-time.
+ */
+#ifdef FILE_MAP_LARGE_PAGES
+#define PG_FILE_MAP_LARGE_PAGES FILE_MAP_LARGE_PAGES
+#else
+#define PG_FILE_MAP_LARGE_PAGES 0
+#endif
+
 HANDLE		UsedShmemSegID = INVALID_HANDLE_VALUE;
 void	   *UsedShmemSegAddr = NULL;
 static Size UsedShmemSegSize = 0;
@@ -216,6 +229,7 @@ PGSharedMemoryCreate(Size size,
 	SIZE_T		largePageSize = 0;
 	Size		orig_size = size;
 	DWORD		flProtect = PAGE_READWRITE;
+	DWORD		desiredAccess;
 
 	ShmemProtectiveRegion = VirtualAlloc(NULL, PROTECTIVE_REGION_SIZE,
 										 MEM_RESERVE, PAGE_NOACCESS);
@@ -258,6 +272,22 @@ PGSharedMemoryCreate(Size size,
 		}
 	}
 
+	/*
+	 * If this binary has been compiled without FILE_MAP_LARGE_PAGES
+	 * available and huge pages are wanted, there is nothing do do
+	 * if running on a version older than 10.0.1703 as this flag is a
+	 * requirement to allow the use of huge pages.
+	 */
+	if (huge_pages == HUGE_PAGES_ON &&
+		PG_FILE_MAP_LARGE_PAGES == 0 &&
+		IsWindowsVersionOrGreater(10, 0, 1703))
+	{
+		ereport(FATAL,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("the processor does not support large pages"),
+				 errdetail("The code has been compiled without huge page support for this version of Windows.")));
+	}
+
 retry:
 #ifdef _WIN64
 	size_high = size >> 32;
@@ -353,12 +383,21 @@ retry:
 	if (!CloseHandle(hmap))
 		elog(LOG, "could not close handle to shared memory: error code %lu", GetLastError());
 
+	desiredAccess = FILE_MAP_WRITE | FILE_MAP_READ;
+
+	/*
+	 * Set large pages if wanted.  FILE_MAP_LARGE_PAGES is needed when
+	 * running at least Windows 10.0.1703.
+	 */
+	if ((flProtect & SEC_LARGE_PAGES) != 0 &&
+		IsWindowsVersionOrGreater(10, 0, 1703))
+		desiredAccess |= PG_FILE_MAP_LARGE_PAGES;
 
 	/*
 	 * Get a pointer to the new shared memory segment. Map the whole segment
 	 * at once, and let the system decide on the initial address.
 	 */
-	memAddress = MapViewOfFileEx(hmap2, FILE_MAP_WRITE | FILE_MAP_READ, 0, 0, 0, NULL);
+	memAddress = MapViewOfFileEx(hmap2, desiredAccess, 0, 0, 0, NULL);
 	if (!memAddress)
 		ereport(FATAL,
 				(errmsg("could not create shared memory segment: error code %lu", GetLastError()),

---

From: Julien Rouhaud <[email-masked]>
Date: Thu, 31 Mar 2022 12:59:08 +0800

Hi,

> the code on a version of Windows that does not support it.

No idea either, and I don't have old enough Windows machine available to try.

> time with version >= 10.0.1703.

That approach seems sensible.  For reference the versionhelpers.h seems to be
available starting with VS 2013 / v120, which is ok since that the oldest
version we support AFAICS.

After *a lot of time* I could finally test this patch.  For the record I could
never find a way to allow 'Lock pages in memory' on the Windows 10 home I have,
so I tried on my Windows 11 evaluation I also had around (version 21H2, so it
should be recent enough).  For the record on this one there was gpedit
available, but then I got a 1450 error, and didn't find any information on how
to reserve huge pages or something like that on Windows.  So I just configured
shared_buffers to 10MB, which should still be big enough to need multiple huge
pages, and it seems to work:

postgres=# select version();
                            version
---------------------------------------------------------------
 PostgreSQL 15devel, compiled by Visual C++ build 1929, 64-bit
(1 row)

postgres=# show huge_pages;
 huge_pages
------------
 on
(1 row)

Now, I also have the exact same result without the patch applied so it's hard
to know whether it had any impact at all.  Unfortunately, I didn't find any
information on how to check if "large pages" are used and/or by which program.

---

From: Michael Paquier <[email-masked]>
Date: Thu, 31 Mar 2022 16:42:37 +0900

> version we support AFAICS.

Hmm.  This points out to a different problem.  The oldest version of
MSVC supported on v10 and v11 is VS2005, so a backpatch means that
those checks would need to be tweaked if we'd want to be perfectly
compatible.  But I'd rather not enter in this game category, limiting
this patch to v12~ :)

> information on how to check if "large pages" are used and/or by which program.

Okano-san has mentioned VMMap upthread:
https://docs.microsoft.com/en-us/sysinternals/downloads/vmmap
--
Michael

---

From: Julien Rouhaud <[email-masked]>
Date: Thu, 31 Mar 2022 16:00:55 +0800

> compatible.  But I'd rather not enter in this game category, limiting

Ah, I indeed haven't check in back branches.  v12 isn't that bad.

> https://docs.microsoft.com/en-us/sysinternals/downloads/vmmap

Yes, I totally missed that.  Thomas Munro also mentioned it off-list, and
also found some reference [1] indicating that large pages should show up as
"Locked WS".  I tested with and without the patch and in both case I don't see
any "Locked WS" usage.  I also get the same Page Table size, which seems
consistent with large pages not being used.  Now, this is a vm running with
virtualbox and we're not entirely sure that huge pages can be allocated with
it.  I wish I could test on my windows 10 machine as it's not virtualized, but
I can't give the required privileges.

[1] https://aloiskraus.wordpress.com/2016/10/03/windows-10-memory-compression-and-more/

---

From: Julien Rouhaud <[email-masked]>
Date: Thu, 31 Mar 2022 18:46:59 +0800

> [1] https://aloiskraus.wordpress.com/2016/10/03/windows-10-memory-compression-and-more/

So, after more digging it turns out that the patch is supposed to work.  If I
force using the PG_FILE_MAP_LARGE_PAGES, postgres starts and I do see "Locked
WS" usage with VMMap, with a size in the order of magnitude of my
shared_buffers.

What is apparently not working on my VM is IsWindowsVersionOrGreater(10, 0,
1703).  I added some debug around to check what GetVersionEx() [2] is saying,
and I get:

dwMajorVersion == 6
dwMinorVersion == 2
dwBuildNumber == 9200

While winver.exe on the same vm says windows 11, version 21H2, build 22000.493.

I'm therefore extremely confused.  The documentation of
IsWindowsVersionOrGreater() at [3] is also highly confusing:

> TRUE if the specified version matches, or is greater than, the version of the
> current Windows OS; otherwise, FALSE.

Isn't that supposed to be the opposite?

[2] https://docs.microsoft.com/en-us/windows/win32/api/sysinfoapi/nf-sysinfoapi-getversionexa
https://docs.microsoft.com/en-us/windows/win32/api/winnt/ns-winnt-osversioninfoexa

[3] https://docs.microsoft.com/en-us/windows/win32/api/versionhelpers/nf-versionhelpers-iswindowsversionorgreater

---

From: Julien Rouhaud <[email-masked]>
Date: Thu, 31 Mar 2022 19:03:09 +0800

So, what GetVersionEx returns is actually "it depends", and this is documented:

> With the release of Windows 8.1, the behavior of the GetVersionEx API has
> changed in the value it will return for the operating system version. The
> value returned by the GetVersionEx function now depends on how the
> application is manifested.
>
> Applications not manifested for Windows 8.1 or Windows 10 will return the
> Windows 8 OS version value (6.2). Once an application is manifested for a
> given operating system version, GetVersionEx will always return the version
> that the application is manifested for in future releases. To manifest your
> applications for Windows 8.1 or Windows 10, refer to Targeting your
> application for Windows.

There's no such indication on IsWindowsVersionOrGreater(), but after seeing
various comments on forums from angry people, it may be a hint that it behaves
similarly.  I'm not sure what to do at this point, maybe just always use the
flag (the PG_ version which may be 0), hoping that hopefully windows won't
define it if it can't handle it?

---

From: Michael Paquier <[email-masked]>
Date: Fri, 1 Apr 2022 14:54:53 +0900

> 1703).  I added some debug around to check what GetVersionEx() [2] is saying,

Okay.  Well, I'd like to think that the patch written as-is is
correct.  Now your tests are saying the contrary, so I don't really
know what to think about it :)

> Isn't that supposed to be the opposite?

I get from the upstream docs that if the runtime version of Windows is
higher than 10.0.1703, IsWindowsVersionOrGreater() should return
true.  Perhaps the issue is in the patch and its argument values, but
it does not look straight-forward to know what those values should
be, and there are no examples in the docs to show that, either :/
--
Michael

---

From: Michael Paquier <[email-masked]>
Date: Fri, 1 Apr 2022 14:59:22 +0900

> define it if it can't handle it?

Looking at the internals of versionhelpers.h, would it work to use as
arguments for IsWindowsVersionOrGreater() the following, in this
order?  As of:
- HIBYTE(_WIN32_WINNT_WINTHRESHOLD)
- LOBYTE(_WIN32_WINNT_WINTHRESHOLD)
- 1703

Just to drop an idea.
--
Michael

---

From: Julien Rouhaud <[email-masked]>
Date: Fri, 1 Apr 2022 14:19:46 +0800

> - LOBYTE(_WIN32_WINNT_WINTHRESHOLD)

I will test that in a bit.  I still think that at least some API exists to give
the real answer since winver and similar report correct info, I just don't know
what those are.

Note that if you want to test yourself you could use this script [1] using the
evaluation virtual machine [2] to automatically setup a windows 11 environment
with everything needed to compile postgres with a extra dependencies.  The
whole process is a bit long though, so I can also give you access to my vm if
you prefer, probably the latency shouldn't be too bad :)

[1] https://github.com/rjuju/pg_msvc_generator/blob/master/bootstrap.ps1
[2] https://developer.microsoft.com/en-us/windows/downloads/virtual-machines/

---

From: Julien Rouhaud <[email-masked]>
Date: Tue, 26 Apr 2022 12:54:35 +0800

Hi,

Please keep the list in copy, especially if that's about Windows specific as
I'm definitely not very knowledgeable about it.

On Fri, Apr 01, 2022 at 09:18:03AM +0000, Wilm Hoyer wrote:
> 
> If you don't wanna go the manifest way, maybe the RtlGetVersion function is the one you need:
> https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/nf-wdm-rtlgetversion?redirectedfrom=MSDN

Thanks for the info!  I tried to use the function but trying to include either
wdm.h or Ntddk.h errors out.  Unfortunately I don't know how to look for a file
in Windows so I don't even know if those files are present.

I searched a bit and apparently some people are using this function directly
opening some dll, which seems wrong.

> Another Idea on windows machines would be to use the commandline to execute
> ver in a separate Process and store the result in a file.

That also seems hackish, I don't think that we want to rely on something like
that.

> >> With the release of Windows 8.1, the behavior of the GetVersionEx API 
> >> has changed in the value it will return for the operating system 
> >> version. The value returned by the GetVersionEx function now depends 
> >> on how the application is manifested.
> >>
> >> Applications not manifested for Windows 8.1 or Windows 10 will return 
> >> the Windows 8 OS version value (6.2). Once an application is 
> >> manifested for a given operating system version, GetVersionEx will 
> >> always return the version that the application is manifested for in 
> >> future releases. To manifest your applications for Windows 8.1 or 
> >> Windows 10, refer to Targeting your application for Windows.
> 
> The documentation is a bit unclear - with the correct functions you should get the:
> Minimum( actualOS-Version, Maximum(Manifested OS Versions))
> The Idea behind, as I understand it, is to better support virtualization and
> backward compatibility - you manifest only Windows 8.1 -> than you always get
> a System that behaves like Windows 8.1 in every aspect.  (Every Aspect not
> true in some corner cases due to security patches)

Well, it clearly does *NOT* behave as a Windows 8.1, even if for some reason
large pages relies on security patches.

Their API is entirely useless, so I'm still on the opinion that we should
unconditionally use the FILE_MAP_LARGE_PAGES flag if it's defined and call it a
day.

---

From: Michael Paquier <[email-masked]>
Date: Wed, 27 Apr 2022 17:13:12 +0900

> opening some dll, which seems wrong.

I was wondering about this whole business, and the manifest approach
is a *horrible* design for an API where the goal is to know if your
run-time environment is greater than a given threshold.

> That also seems hackish, I don't think that we want to rely on something like

Hmm.  That depends on the dependency set, I guess.  We do that on
Linux at some extent to for large pages in sysv_shmem.c.  Perhaps this
could work for Win10 if this avoids the extra loopholes with the
manifests.

> Their API is entirely useless,

This I agree.

> so I'm still on the opinion that we should

Are we sure that this is not going to cause failures in environments
where the flag is not supported?
--
Michael

---

From: Michael Paquier <[email-masked]>
Date: Fri, 8 Jul 2022 07:38:49 +0900

> unconditionally use the FILE_MAP_LARGE_PAGES flag if it's defined and call it a

Now that the minimal runtime version is Windows 10 in v16~ thanks to
495ed0e, we could be much more aggressive and do the attached, which
is roughly what Thomas has proposed upthread at the exception of
assuming that FILE_MAP_LARGE_PAGES always exists, because updates are
forced by MS in this environment.  We could make it conditional, of
course, with an extra #ifdef painting.
--
Michael
From 9a1450ecbe291cfc92e97f49506db9f443c8cbe9 Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email-masked]>
Date: Thu, 7 Jul 2022 16:51:18 +0900
Subject: [PATCH v2] Fix huge_pages on current Windows.

Since Windows 10 1703, it's necessary to pass a flag to MapViewOfFile()
to enable large pages at map time.

Reported-by: Okano Naoki
Author: Thomas Munro
Discussion: https://postgr.es/m/17448-0a96583a67edb1f7%40postgresql.org
---
 src/backend/port/win32_shmem.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/backend/port/win32_shmem.c b/src/backend/port/win32_shmem.c
index 6cf69411db..2f51da5a6a 100644
--- a/src/backend/port/win32_shmem.c
+++ b/src/backend/port/win32_shmem.c
@@ -216,6 +216,7 @@ PGSharedMemoryCreate(Size size,
 	SIZE_T		largePageSize = 0;
 	Size		orig_size = size;
 	DWORD		flProtect = PAGE_READWRITE;
+	DWORD		desiredAccess;
 
 	ShmemProtectiveRegion = VirtualAlloc(NULL, PROTECTIVE_REGION_SIZE,
 										 MEM_RESERVE, PAGE_NOACCESS);
@@ -353,12 +354,17 @@ retry:
 	if (!CloseHandle(hmap))
 		elog(LOG, "could not close handle to shared memory: error code %lu", GetLastError());
 
+	desiredAccess = FILE_MAP_WRITE | FILE_MAP_READ;
+
+	/* Set large pages if wanted. */
+	if ((flProtect & SEC_LARGE_PAGES) != 0)
+		desiredAccess |= FILE_MAP_LARGE_PAGES;
 
 	/*
2.36.1

---

From: Michael Paquier <[email-masked]>
Date: Fri, 16 Sep 2022 21:51:40 +0900

> 1703 kernel sees this flag, does it reject it or ignore it?

I have been looking at this thread, and found an answer in an example
of application creating a map object with large pages in [1]:
"This flag is ignored on OS versions before Windows 10, version 1703."

So based on that I think that we could just apply and backpatch what
you have here.  This issue is much easier to reason about on HEAD
where we just care about Win >= 10, and we've be rather careful with
changes like that when it came to Windows.  Any objections about doing 
a backpatch?  I'd like to do so after an extra lookup, if there are no
objections.  Or would folks prefer a HEAD-only fix for now?

[1]: https://docs.microsoft.com/en-us/windows/win32/memory/creating-a-file-mapping-using-large-pages?source=recommendations
--
Michael

---

From: Tom Lane <[email-masked]>
Date: Fri, 16 Sep 2022 10:29:38 -0400

> objections.  Or would folks prefer a HEAD-only fix for now?

Let's just fix it in HEAD.  I think the risk/reward ratio isn't very
good here.

(I'd be particularly against changing this in v10, because 10.23 will
be the last one; there will be no second chance if we ship it broken.)

			regards, tom lane

---

From: Julien Rouhaud <[email-masked]>
Date: Fri, 16 Sep 2022 22:36:12 +0800

> be the last one; there will be no second chance if we ship it broken.)

+1

---

From: Michael Paquier <[email-masked]>
Date: Sat, 17 Sep 2022 15:41:11 +0900

>> be the last one; there will be no second chance if we ship it broken.)

Okay, fine by me.  I have applied that only on HEAD, then.
--
Michael