Hey,
On 30/10/12 20:33, Andres Freund wrote:
> +#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...
Yeah, looks strange to me, too. This is why I asked if this is really
all we have to do.
> 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.
Good point. I will change this to the smallest available page size.
> 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.
Ok, point taken.
> […]
> + HUGE_TLB_TRY, huge_tlb_options,
> + NULL, NULL, NULL
> + },
>
> you use HUGE_TLB_TRY with ifdef here.
Ah, right. Setting it to HUGE_TLB_OFF when no MAP_HUGETLB is available.
Greetings,
CK
