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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to