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