Re: Re: [HACKERS] Patch für MAP_HUGETLB for mmap() shared memory

Hi CK,

On 2012-10-30 21:16:07 +0100, Christian Kruse wrote:
> index b4fcbaf..66ed10f 100644
> --- a/doc/src/sgml/config.sgml
> +++ b/doc/src/sgml/config.sgml

I think a short introduction or at least a reference on how to configure
hugepages would be a good thing.

> <varlistentry id="guc-temp-buffers" xreflabel="temp_buffers">
> <term><varname>temp_buffers</varname> (<type>integer</type>)</term>
> <indexterm>
> diff --git a/src/backend/port/sysv_shmem.c b/src/backend/port/sysv_shmem.c
> index df06312..f9de239 100644
> --- a/src/backend/port/sysv_shmem.c
> +++ b/src/backend/port/sysv_shmem.c
> @@ -27,10 +27,14 @@
> #ifdef HAVE_SYS_SHM_H
> #include <sys/shm.h>
> #endif
> +#ifdef MAP_HUGETLB
> +#include <dirent.h>
> +#endif

I think a central #define for the MAP_HUGETLB capability would be a good
idea, akin to HAVE_SYS_SHM_H.

E.g. this:
> --- a/src/backend/utils/misc/guc.c
> +++ b/src/backend/utils/misc/guc.c
> @@ -22,6 +22,7 @@
> #include <limits.h>
> #include <unistd.h>
> #include <sys/stat.h>
> +#include <sys/mman.h>
> #ifdef HAVE_SYSLOG
> #include <syslog.h>
> #endif

is unlikely to fly on windows.

> +/*
> + * static long InternalGetHugepageSize()
> + *
> + * Attempt to get a valid hugepage size from /sys/kernel/mm/hugepages/ by
> + * reading directory contents
> + * Will fail (return -1) if the directory could not be opened or no valid
> + * page sizes are available. Will return the biggest hugepage size on
> + * success.
> + *
> + */

The "biggest" remark is out of date.

> +static long
> +InternalGetHugepageSize()
> +{
> ...
> + if ((smallest_size == -1 || size < smallest_size)
> + && InternalGetFreeHugepagesCount(ent->d_name) > 0)
> + {
> + smallest_size = size;
> + }
> ...
> +
> + if (smallest_size == -1)
> + {
> + ereport(huge_tlb_pages == HUGE_TLB_TRY ? DEBUG1 : WARNING,
> + (errmsg("Could not find a valid hugepage size"),
> + errhint("This error usually means that either CONFIG_HUGETLB_PAGE "
> + "is not in kernel or that your architecture does not "
> + "support hugepages or you did not configure hugepages")));
> + }

I think differentiating the error message between no hugepages found and
InternalGetFreeHugepagesCount(ent->d_name) always beeing zero would be a
good idea. Failing this way if
InternalGetFreeHugepagesCount(ent->d_name) < 0 seems fine.

Greetings,

Andres Freund