Re: [PATCH v3 2/6] nptl: Add POSIX-proposed sem_clockwait

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

On 26/06/2019 10:47, Mike Crowe wrote:
> On Wednesday 26 June 2019 at 15:02:44 +0200, Florian Weimer wrote:
>> * Adhemerval Zanella:
>>
>>> On 25/06/2019 18:39, Florian Weimer wrote:
>>>> * Mike Crowe:
>>>>
>>>>> On Tuesday 25 June 2019 at 20:06:53 +0200, Florian Weimer wrote:
>>>>>> * Mike Crowe:
>>>>>>
>>>>>>> On Wednesday 05 June 2019 at 18:07:18 -0300, Adhemerval Zanella wrote:
>>>>>>>> On 27/05/2019 17:03, Mike Crowe wrote:
>>>>>>>>> Add:
>>>>>>>>>
>>>>>>>>>  int sem_clockwait (sem_t *sem, clockid_t clock, const struct timespec *abstime)
>>>>>>>>>
>>>>>>>>> which behaves just like sem_timedwait, but measures abstime against the
>>>>>>>>> specified clock. Currently supports CLOCK_REALTIME and CLOCK_MONOTONIC and
>>>>>>>>> sets errno == EINVAL if any other clock is specified.
>>>>>>>>
>>>>>>>> For non-POSIX definition we will need to first export it as a GNU extension
>>>>>>>> with a possible non-reserved name and later, when it is included on the 
>>>>>>>> standard, to add an alias to the expected name.  
>>>>>>>>
>>>>>>>> The usual naming scheme is to append the _np suffix (non-portable) on 
>>>>>>>> implementation, similar to recent posix_spawn file action extensions.
>>>>>>>> In this case it would be sem_clockwait_np.
>>>>>>
>>>>>>> I thought we went through this before[1] and agreed that the _np suffix wasn't
>>>>>>> necessary in this case. However, I couldn't find a reply to Florian
>>>>>>> agreeing to that.
>>>>>>
>>>>>> Upon second thought, I agree with Adhemerval here.  We should use a
>>>>>> function with the _np suffix, otherwise we'll be in a world of pain if
>>>>>> something else is standardized.
>>>>>
>>>>> Well, the plan is to standardise this[1] and The Austin Group are the ones
>>>>> who decided on the names, but there needs to be an implementation before
>>>>> that can happen. Presumably that implementation can have _np suffixes in
>>>>> the short term though.
>>>>>
>>>>> I shall apply some sed to the patch series and repost.
>>>>
>>>> Sorry, I see that we don't use _np for the other functions.
>>>> I'm confused now. 8-(
>>>>
>>>> Adhemerval, is there value in keeping this consistent?  Have these
>>>> functions progressed to different stages in the POSIX process?
>>>
>>> My understanding of Austin proposal [1] (Mike can correct me) is naming is
>>> settled and Android will also probably follow the same naming scheme. However, 
>>> since Austin Group does not really propose newer interfaces, they
>>> required a actual implementation to move forward.
>>>
>>> Joseph also added sometime ago on IRC that we should add the symbols with 
>>> the expected naming, but keep in mind there might be future incompatibilities 
>>> requiring symbol versioning.
>>>
>>> [1] http://austingroupbugs.net/view.php?id=1216
>>
>> I'm still puzzled what's different compared to
>> posix_spawn_file_actions_addfchdir_np or pthread_rwlock_clockrdlock.
>> Are they at different stages of the POSIX process?
> 
> I'm not sure what stage posix_spawn_file_actions_addfchdir (which appears
> to be http://austingroupbugs.net/view.php?id=1208 ) is at, but it looks
> like Eric Blake added it with the _np suffix prior to seeking
> standardisation.

My guess is to make it follow Solaris11 implementation which added the symbol
with _np suffix. The POSIX was accepte with the expected name, so eventually
both Solaris and glibc will need to provide the symbol with the expected
name.

> 
> I attended an Austin Group teleconference (minutes at
> https://www.opengroup.org/austin/docs/austin_895.txt ) late last year and I
> believe that there was consensus in favour. We haven't yet determined which
> track to use to get the functions into POSIX - I've been focusing on the
> implementation.
> 
> If people would feel happier with using _np suffixes for now, then I think
> we should add them. However, my next step after this series lands (if it
> does) is to get a change into libstdc++ to use pthread_cond_clockwait{,_np}
> so we'd have to keep the _np versions around for quite a while even after
> the non-_np versions are standardised. libstdc++ would need to support
> either name in parallel for a while too.

Once it set on libc abi we can't remove without bump the major version (and
this is troublesome and it would require time). I would prefer to not add
with _np just because it will most likely add needless complexity on 
libstdc++ side (or any other project) to check for multiples names for the 
same functionality.

> 
> If it would be sufficient, we could delay landing the series while I try
> confirming with The Austin Group that they are definitely happy with the
> naming and parameters of the new functions. (There's a meeting tomorrow,
> and I may be able to get it on the agenda.)

I don't have a strong opinion, but I think it would be good to land on 2.30.
I have the remaining patchset set for inclusion in my user branch and from 
Austin Group procedures I think is unlikely they will change the naming, 
specially for a non default _np or even after glibc provides the required 
actual implementation.