[PATCH] support: Skip malloc hugetlb={1, 2} variants when kernel cannot honor them

[Adhemerval Zanella Netto]

On 26/05/26 18:52, DJ Delorie wrote:
> Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
>> diff --git a/malloc/Makefile b/malloc/Makefile
>> index f95bca7f8fa..72aa77af206 100644
>> --- a/malloc/Makefile
>> +++ b/malloc/Makefile
>> @@ -351,6 +351,19 @@ $(objpfx)%-threaded-worker.o: CPPFLAGS += -DTEST_IN_THREAD=TEST_THREAD_WORKER
>>  $(objpfx)%-threaded-worker.o: %.c $(before-compile)
>>  	$(compile-command.c)
>>  
>> +# Compile the hugetlb={1,2} test variants from their base sources with
>> +# -DTEST_HUGETLB so support/test-driver.c gates them on kernel support.
> 
> We really should have called that test-driver.h or something; at first
> glance I thought "that can't work" but the .c file is always #included.
> Sigh.
> 
>> diff --git a/support/support_check_hugetlb.c b/support/support_check_hugetlb.c
> 
>> +bool
>> +support_thp_is_madvise (void)
>> +{
>> +#ifdef __linux__
> 
> There's nothing in here that is linux-specific at compile time, might as
> well leave it unconditional.

Alright, although the sysfs is really a Linux interface.

> 
>> +  FILE *fp = fopen ("/sys/kernel/mm/transparent_hugepage/enabled", "r");
>> +  if (fp == NULL)
>> +    return false;
>> +  char line[256];
>> +  bool madvise = false;
>> +  if (fgets (line, sizeof (line), fp) != NULL)
> 
> Could use fread instead; the whole file fits in the buffer and fread
> allows you to avoid buffer overflow.

Ack, I will used something similar to __get_thp_mode.

> 
>> +bool
>> +support_hugepages_reserved (void)
>> +{
>> +#ifdef __linux__
> 
> Same here, nothing in this is Linux-specific at compile time.
> 
>> +/* Returns true if TUNABLES (a GLIBC_TUNABLES-style colon-separated list)
>> +   contains an entry of the form NAME=VALUE.  */
> 
> Why can't you just use tunable_get() ?  That would avoid complex parsing
> code and support future (i.e. current ;) efforts to pull in tunables
> from other sources than the environment variable.  Yes, I know we expect
> this from the Makefile, but still, tunable_get() alrady does what you
> want.

I tried to avoid using GLIBC_PRIVATE and other private interfaces on libsupport,
but TUNABLE_GET_FULL does simplify things.

> 
>> +static bool
>> +tunable_equals (const char *tunables, const char *name, const char *value)
> 
>> +void
>> +support_check_malloc_hugetlb (void)
>> +{
>