Re: [HACKERS] Supporting huge pages on Windows

On Fri, Dec 1, 2017 at 5:02 AM, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
wrote:

> On Fri, Nov 24, 2017 at 9:28 AM, Tsunakawa, Takayuki
> <tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com> wrote:
> > From: Thomas Munro [mailto:thomas(dot)munro(at)enterprisedb(dot)com]
> >> I hope Tsunakawa-san doesn't mind me posting another rebased version of
> >> his patch. The last version conflicted with the change from SGML to XML
> >> that just landed in commit 3c49c6fa.
> >
> > Thank you very much for keeping it fresh. I hope the committer could
> have some time.
>
> I have moved this patch to next CF. Magnus, you are registered as a
> reviewer of this patch. Are you planning to look at it and potentially
> commit it?
>

Apologies for the delays here. Yes, I was and am.

I took a look at this patch again. I've made some small updates to comments
etc. There was also from what I can tell one actual bug in the code -- a
missing free() of delPrivs.

The debug message for ERROR_NO_SYSTEM_RESOURCES said it turned off huge
pages because of not enough huge pages, but AIUI it can be because of other
resources as well. So I dropped that specific.

However, reading through a few things -- we now use the
API GetLargePageMinimum() unconditionally. This API appeared in Windows
Vista and Windows 2003. That means that if we apply this patch, those are
now our minimum versions of Windows. At least unless Microsoft backpatched
things.

This is probably equally true of some other things we do, but those are
runtime, and we would just give an error on old platforms. If I'm thinking
right, then direct linking to GetLargePageMinimum() will simply make it
impossible to start a PostgreSQL backend with or without huge pages on
previous versions, because it will give a linker error.

I wonder if we need to do something similar to what we already do for
CreateRestrictedToken() in pg_ctl.c for example. That one actually is done
for compatibility with NT4/2000 -- CreateRestrictedToken was added in XP.
So while we could consider getting rid of that workaround now, perhaps we
need to put a similar one in place for this?

I don't have a Windows build box around ATM, or a Windows XP, so if
somebody could try the attached version, build a postgres and see if it
even starts on a Windows XP machine, that would be useful input!

If we can confirm/deny the situation around XP and decide what to do about
it, I am now happy to commit the rest of this patch.

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>