Re: Re: [HACKERS] Patch für MAP_HUGETLB for mmap() shared memory - Mailing list pgsql-hackers

From Andres Freund
Subject Re: Re: [HACKERS] Patch für MAP_HUGETLB for mmap() shared memory
Date
Msg-id 201210302033.18558.andres@2ndquadrant.com
Whole thread Raw
In response to Re: Re: [HACKERS] Patch für MAP_HUGETLB for mmap() shared memory  (Christian Kruse <cjk+postgres@defunct.ch>)
Responses Re: Re: [HACKERS] Patch für MAP_HUGETLB for mmap() shared memory  (Christian Kruse <cjk+postgres@defunct.ch>)
Re: Re: [HACKERS] Patch für MAP_HUGETLB for mmap() shared memory  (Christian Kruse <cjk+postgres@defunct.ch>)
Re: Re: [HACKERS] Patch für MAP_HUGETLB for mmap()shared memory  (Christian Kruse <cjk+postgres@defunct.ch>)
Re: Re: [HACKERS] Patch für MAP_HUGETLB for mmap() shared memory  (Christian Kruse <cjk@wwwtech.de>)
List pgsql-hackers
On Tuesday, October 30, 2012 08:20:33 PM Christian Kruse wrote:
> Hey,
>
> Oh man, first I didn't sent the email to the list and now I forgot the
> attachment. I should really get some sleep, sorry for any
> inconveniences :(

+#ifdef MAP_HUGETLB
+#  ifdef __ia64__
+#    define PG_HUGETLB_BASE_ADDR (void *)(0x8000000000000000UL)
+#    define PG_MAP_HUGETLB (MAP_HUGETLB|MAP_FIXED)
+#  else

Not your fault, but that looks rather strange to me. The level of documentation
around the requirement of using MAP_FIXED is subpar...

+long
+InternalGetHugepageSize()
+{
+...
+            if (biggest_size < size && InternalGetFreeHugepagesCount(ent-
>d_name) > 0)
+            {
+                biggest_size = size;
+            }

I dislike automatically using the biggest size here. There are platforms with
16GB hugepages... I think we should rather always use the smallest size and
leave it to the kernel to choose whatever pagesize he wants to use.

+#ifdef MAP_HUGETLB
+        long    pagesize = InternalGetHugepageSize();
+
+        if (pagesize <= 0)
+            pagesize = sysconf(_SC_PAGE_SIZE);
+#else
         long    pagesize = sysconf(_SC_PAGE_SIZE);
+#endif

Thats going to log two warnings if the current system doesn't have hugepage
support compiled in and thus no /sys/kernel/mm/hugepages directory.
I think you should 1. only test this if TRY or ON is configured, 2. make the
messages in InternalGetHugepageSize DEBUG1 at least for the TRY case.

+    {
+        {"huge_tlb_pages",
+#ifdef MAP_HUGETLB
+            PGC_SUSET,
+#else
+            PGC_INTERNAL,
+#endif
+            RESOURCES_MEM,
+            gettext_noop("Enable/disable the use of the hugepages feature"),
+            NULL
+        },
+        &huge_tlb_pages,
+        HUGE_TLB_TRY, huge_tlb_options,
+        NULL, NULL, NULL
+    },

you use HUGE_TLB_TRY with ifdef here.


Looking forward to having this...

Andres
--
 Andres Freund                       http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services



pgsql-hackers by date:

Previous
From: Christian Kruse
Date:
Subject: Re: Re: [HACKERS] Patch für MAP_HUGETLB for mmap() shared memory
Next
From: Christian Kruse
Date:
Subject: Re: Re: [HACKERS] Patch für MAP_HUGETLB for mmap() shared memory