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

[Florian Weimer <fweimer at redhat dot com>]

* 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.

Thanks,
Florian