[PATCH] Linux: Avoid calling malloc indirectly from __get_nprocs

[Adhemerval Zanella]

On 06/07/2021 09:50, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>>> +/* Compute the population count of the entire array.  */
>>> +static int
>>> +__get_nprocs_count (const unsigned long int *array, size_t length)
>>> +{
>>> +  int count = 0;
>>> +  for (size_t i = 0; i < length; ++i)
>>> +    if (__builtin_add_overflow (count,  __builtin_popcountl (array[i]),
>>> +				&count))
>>> +      return INT_MAX;
>>> +  return count;
>>> +}
>>
>> Could we avoid replicate the same logic over different files? We have a
>> countbits() on posix/sched_cpucount.c, so I think it would better to move
>> it on a sched.h and use it instead (there is no need to really handle
>> overflow here, it would require a *very* large buffer...).
> 
> Probably, yes.
> 
> Should I send a patch?  I was busy with other stuff last week, but I
> should be able to work on fixing this now.

I have this one m backlog, I will prob send a patch this week.

> 
>>> +/* __get_nprocs with a large buffer.  */
>>> +static int
>>> +__get_nprocs_large (void)
>>> +{
>>> +  /* This code cannot use scratch_buffer because it is used during
>>> +     malloc initialization.  */
>>> +  size_t pagesize = GLRO (dl_pagesize);
>>> +  unsigned long int *page = __mmap (0, pagesize, PROT_READ | PROT_WRITE,
>>> +				    MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
>>> +  if (page == MAP_FAILED)
>>> +    return 2;
>>> +  int r = INTERNAL_SYSCALL_CALL (sched_getaffinity, 0, pagesize, page);
>>> +  int count;
>>> +  if (r > 0)
>>> +    count = __get_nprocs_count (page, pagesize / sizeof (unsigned long int));
>>> +  else if (r == -EINVAL)
>>> +    /* One page is still not enough to store the bits.  A more-or-less
>>> +       arbitrary value.  This assumes t hat such large systems never
>>> +       happen in practice.  */
>>> +    count = GLRO (dl_pagesize) * CHAR_BIT;
>>> +  else
>>> +    count = 2;
>>> +  __munmap (page, GLRO (dl_pagesize));
>>
>> Maybe use pagesize here since you are defining it.
> 
> Right.
> 
>> I would prefer that since now we don't iterate increasing the buffer
>> size for sched_getaffinity we go for a simplified version and use a
>> large buffer instead.
>>
>> Linux currently supports at maximum of 4096 cpus for most
>> architectures:
> 
> I'm not sure if that's a good idea.  I think some distributions patched
> the defaults in the past.  The limit isn't really set in stone.

But we are still using an artificial limit here, although higher.  Couldn't
we do the other way around: make malloc use sched_getaffinity with a
limited buffer and use either to the saturated value or 2 if syscall failed,
and get back to use a scratch_buffer on libc provided symbol (which does not
have any limit and simpler to handle than stack plus mmap)? For malloc it 
does not seems critical to get the fully correct CPU number for large systems.