Re: Re: [HACKERS] Patch für MAP_HUGETLB for mmap() shared memory

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