Re: [PATCH] Use MAP_HUGETLB where supported (v3)

Hi,

On 15/11/13 15:17, Heikki Linnakangas wrote:
> I spent some time whacking this around, new patch version attached. I moved
> the mmap() code into a new function, that leaves the PGSharedMemoryCreate
> more readable.

I think there's a bug in this version of the patch. Have a look at
this:

+    if (huge_tlb_pages == HUGE_TLB_ON || huge_tlb_pages == HUGE_TLB_TRY)
+    {
[…]
+        ptr = mmap(NULL, *size, PROT_READ | PROT_WRITE,
+                   PG_MMAP_FLAGS | MAP_HUGETLB, -1, 0);
[…]
+    }
+#endif
+
+    if (huge_tlb_pages == HUGE_TLB_OFF || huge_tlb_pages == HUGE_TLB_TRY)
+    {
+        allocsize = *size;
+        ptr = mmap(NULL, *size, PROT_READ | PROT_WRITE, PG_MMAP_FLAGS, -1, 0);
+    }

This will lead to a duplicate mmap() if hugepages work and
huge_tlb_pages == HUGE_TLB_TRY, or am I missing something?
I think it should be like this:
if (huge_tlb_pages == HUGE_TLB_OFF ||    (huge_tlb_pages == HUGE_TLB_TRY && ptr == MAP_FAILED))

Best regards,

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