Re: [PATCH v3] nptl: Deallocate the thread stack on setup failure (BZ #19511)

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 09/06/2021 10:49, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> THREAD_RAN only signals if whether clone() or thread setup has failed, 
>> there is no synchronization involved and it is already local to
>> creating thread.  I changed the concurrency comment to the following,
>> to indicate that (c) and (d) are essentially the same as you noted:
>>
>>    (c) If either a joinable or detached thread setup failed and THREAD_RAN
>>        is true, then the creating thread releases ownership to the new thread,
>>        the created thread sees the failed setup through PD SETUP_FAILED
> 
> PD->setup_failed (I think this is the GNU convention).

Ack (I was in doubt which would the rule here).

> 
>>        member, releases the PD ownership, and exits.  The creating thread will
>>        be responsible for cleanup the allocated resources.  The THREAD_RAN is
>>        local to creating thread and indicate whether thread creation or setup
>>        has failed.
>>    
>>    (d) If the thread creation failed and THREAD_RAN is false (meaning
>>        ARCH_CLONE has failed), then the creating thread retains ownership
>>        of PD and must cleanup he allocated resource.  No waiting for the new
>>        thread is required because it never started.
> 
> Yes, that's much better.
> 
>> I think for such case you will need to set PD->stopped_start to true
>> before thread creation (similar to what affinity and schedule set do),
>> add the required initialization after the setup failure check, and
>> handle potential errors.
>>
>> Something like (I removed the comments to simplify):
> 
> Yeah, that is what I expect as well.
> 
>> Below it is the updated patch based on your suggestion, including the
>> concurrency state descriptor regarding THREAD_RAN.  Are this version
>> ok to commit?
> 
> This version looks okay to me, with the PD->setup_failed change above.

Thanks, I will run a check on some architecture and commit the patchset.