Re: [PATCH] nptl: Remove cancellation checks from sem_{timed}wait (BZ #23006)

[Adhemerval Zanella <adhemerval dot zanella at linaro dot org>]

On 04/07/2019 11:19, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> On 03/07/2019 15:28, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>>
>>>> On 25/06/2019 11:29, Adhemerval Zanella wrote:
>>>>>
>>>>>
>>>>> On 25/06/2019 10:36, Florian Weimer wrote:
>>>>>> * Adhemerval Zanella:
>>>>>>
>>>>>>> In any case, do you think we should make sem_{timed}wait not a cancellation
>>>>>>> entrypoint for 2.30? I can rework the patch if it is the case.
>>>>>>
>>>>>> Do we have any other form of blocking synchronization that is
>>>>>> cancellable?  If not, a cancellable semaphore wait operation would make
>>>>>> sense.  But then we should perform the cancellation check on the fast
>>>>>> path, too.
>>>>>
>>>>> pthread_cond_wait, pthread_cond_timedwait, and pthread_join as well the
>>>>> gnu extension pthread_timedjoin_np and pthread_cond_clockwait (not yet
>>>>> upstream). Depending of you definition of synchronization, you can also
>>>>> include sigtimedwait, sigwait, sigwaitinfo, wait, waitid, and waitpid.
>>>>>
>>>>> We explicit does not support cancellation for pthread_rwlock_rdlock,
>>>>> pthread_rwlock_timedrdlock, pthread_rwlock_timedwrlock, and
>>>>> pthread_rwlock_wrlock. I tend to see that pthread_rwlock_* are similar
>>>>> to pthread_mutex and the rationale layout on 'Thread Cancellation 
>>>>> Overview' also applies.
>>>>>
>>>>> So at least we have synchronization functions that explicit does not
>>>>> act for cancellation, we might extend it to sem_{timed}wait now that
>>>>> they are also listed on 'shall'.
>>>>>
>>>>> However, I still think this does not really follow along with the
>>>>> rationale exposed on the very issue that triggered it [1], which aimed
>>>>> imho to just remove the requirement of check for cancellation on fast
>>>>> path only. 
>>>>>
>>>>> [1] http://austingroupbugs.net/view.php?id=1076#c3938
>>>>>
>>>>
>>>> Florian, how should we proceed regarding it? Should we just move
>>>> sem_{timed}wait out of the cancellable entrypoints or act uppon just
>>>> for the potentially blocked case? For latter do you still think we 
>>>> should keep a compat symbol for early check?
>>>
>>> Ugh, to be honest, I don't think any code change is required here.
>>
>> By 'here' do you mean the patch or current implementation? I still think
>> we can use the Austin clarification to optimize the sem_{timed}wait
>> and remove the early checks for the non-blocked case. And I also don't
>> think it is worth to add a compatibility check for this specific
>> semantic.
> 
> I think the optimization is invalid, for the reasons I outlined earlier.

This discussion seems to be on stalemate and I think this is not really
a blocker for 2.30. I will remove it from the list and we can sort this
when 2.31 opens.