Re: [PATCH] nptl: Add pthread_thread_number_np function

[Florian Weimer <fweimer@redhat.com>]

On 12/21/2017 10:26 AM, Carlos O'Donell wrote:

>> +The returned number is only unique with regards to the current process.
>> +It may be shared by subprocesses and other processes in the system.
>> +
>> +The initial (main) thread has number 1.  Thread numbers are not
>> +necessarily assigned in a consecutive fashion.  They bear no
>> +relationship to POSIX thread IDs (@code{pthread_t} values), process IDs
>> +or thread IDs assigned by the kernel.
> 
> I would like us to add something like this:
> ~~~
> While the return type of this function is only 64-bits wide, the intent
> is not to impose an artificial limit on the number of threads that can be
> created by the runtime. In the future this interface may be extended
> to 128-bits to support creating as many threads as a user may need
> for the lifetime of the process.
> ~~~
> 
> That way the intent of the interface and future changes are clear.

So how would a programmer use this interface in a future-proof way?  I 
think such a statement would raise more questions than it answers.

>> diff --git a/nptl/allocatestack.c b/nptl/allocatestack.c
>> index 1cc7893195..454df7740b 100644
>> --- a/nptl/allocatestack.c
>> +++ b/nptl/allocatestack.c
>> @@ -413,16 +413,28 @@ allocate_stack (const struct pthread_attr *attr, struct pthread **pdp,
>>     assert (powerof2 (pagesize_m1 + 1));
>>     assert (TCB_ALIGNMENT >= STACK_ALIGN);
>>   
>> -  /* Get the stack size from the attribute if it is set.  Otherwise we
>> -     use the default we determined at start time.  */
>> -  if (attr->stacksize != 0)
>> -    size = attr->stacksize;
>> -  else
>> -    {
>> -      lll_lock (__default_pthread_attr_lock, LLL_PRIVATE);
>> +  uint64_t thread_number;
>> +  lll_lock (__default_pthread_attr_lock, LLL_PRIVATE);
>> +  {
>> +    /* Number 1 is reserved for the initial thread.  Reuse
>> +       __default_pthread_attr_lock to avoid concurrent updates of this
>> +       counter.  */
> 
> OK.
> 
>> +    static uint64_t global_thread_number = 1;
>> +    thread_number = ++global_thread_number;
> 
> Alright, here comes serious worry #1.
> 
> If we say "Thread numbers are not necessarily assigned in a consecutive fashion.",
> and we assign them in a consecutive fashion, users will ignore this statement
> and use what empirically appears to be true.

The above does not actually assign thread numbers in a consecutive 
fashion, from an application perspective because the implementation can 
create its own threads for its own internal use.  (librt and libanl do 
this.)

> People start relying on this counter incrementing from 1 upwards.
> 
> People start using this monotonic property for indexing.
> 
> Soon we can't change it because it's implied API behaviour.
> 
> I think we should disabuse them from doing something low cost to roll the value:
> 
> * Do nothing for thread 1, leaving it 1.
> * Check global_thread_number for overflow instead.
> * Pick a random number of bits to roll between 0-63 (picked at process startup)
> * Roll the value by some that number of bits.
> * Use the rolled value as the thread_number

Not sure if I understand this.  Do you want us to start at a random 
value?  Or assign IDs randomly?  The latter will have a collision much 
sooner.

I can switch the thread numbers to a fixed, but random-looking 
permutation of the integers in [0, 2**64), but this looks excessive.

In my opinion, we need to assume at one point that programmers read the 
documentation.

Thanks,
Florian