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