What clocks are supported by pthread_clockjoin_np()

[Adhemerval Zanella]

On 20/11/2020 11:22, Mike Crowe wrote:
> On Friday 20 November 2020 at 09:40:48 -0300, Adhemerval Zanella wrote:
>>
>>
>> On 20/11/2020 06:10, Mike Crowe wrote:
>>> On Thursday 19 November 2020 at 09:29:55 -0300, Adhemerval Zanella via Libc-alpha wrote:
>>>>
>>>>
>>>> On 19/11/2020 09:00, Mike Crowe via Libc-alpha wrote:
>>>>> On Thursday 19 November 2020 at 09:42:07 +0100, Michael Kerrisk (man-pages) wrote:
>>>>>> I was looking at adding manual page documentation for
>>>>>> pthread_clockjoin_np(), but it's not clear to me from the code what
>>>>>> clocks are supported by the API, and the glibc info docs seem to be
>>>>>> silent on this point. What clocks are supported?
>>>>>
>>>>> That's an interesting question. My intention was that it would support
>>>>> CLOCK_REALTIME and CLOCK_MONOTONIC just like pthread_cond_clockwait,
>>>>> sem_clockwait etc.
>>>>
>>>> I think it does make sense to align with other pthread/rt implementations.
>>>
>>> OK. I shall prepare a patch to reject anything other than CLOCK_REALTIME
>>> and CLOCK_MONOTONIC.
>>>
>>>>> However, since the current implementation currently just calls
>>>>> clock_gettime to calculate a relative timeout to pass to futex it will work
>>>>> with any clock that clock_gettime supports.
>>>>>
>>>>> Perhaps we ought to document pthread_clockjoin_np as supporting only
>>>>> CLOCK_REALTIME and CLOCK_MONOTONIC and then change the implementation to
>>>>> fail with EINVAL on any other clocks? Doing this means that the
>>>>> implementation can switch to passing an absolute timeout to futex in the
>>>>> future, which would mean that warping of CLOCK_REALTIME would be honoured
>>>>> correctly by the kernel (although it's not clear to me how important that
>>>>> really is to anyone.)
>>>>
>>>> There is another related issue on which error to return for clock not
>>>> available to used with the syscall, for instance pthread_mutex_clocklock/PI 
>>>> with CLOCK_MONOTONIC (BZ #26801 [1]). My take is to return ENOTSUP, however
>>>> as Florian has raised, I am not sure if this is the correct return code.
>>>>
>>>> [1] https://sourceware.org/pipermail/libc-alpha/2020-October/119211.html
>>>
>>> The wording[2] destined for POSIX says to return EINVAL in this situation.
>>> I originally tried to return different errors for different types of
>>> incorrect clock[3], but I thought that doing so would be prone to errors
>>> and no-one seemed to disagree.
>>>
>>> (I don't read libc-alpha regularly, so feel free to email me directly if
>>> this topic comes up again.)
>>>
>>> Thanks.
>>>
>>> Mike.
>>>
>>> [2] https://www.austingroupbugs.net/view.php?id=1216#c4478
>>> [3] https://www.austingroupbugs.net/view.php?id=1216#c4196
>>>
>>
>> Thanks for the point, if I understood correctly this defect it is still
>> open. My point of using ENOTSUP is to align with clock_nanosleep,
>> pthread_mutexattr_setprotocol, pthread_setschedparam, mmap, among
>> other with does return ENOTSUP for input combination for valid, but
>> not supported clocks.
> 
>> On glibc itself, pthread_mutex_init returns ENOTSUP for robust priority
>> protected mutexes and also adds a runtime check if __NR_set_robust_list
>> syscall is not support (indicating no mutex robust support).  The later
>> is not supported only for specific chips which lacks some atomic
>> instructions the kernel require to provide robust futex support.
> 
> I believe that the wording has been incorporated into the document to be
> used to update the draft of the next POSIX standard, but it's still not too
> late to change it. It's in the document referenced by
> https://www.mail-archive.com/austin-group-l@opengroup.org/msg07142.html

Ok, good to know.

> 
> I do see the benefit of having a distinct error code for "you've passed a
> valid clock, but it's not one that this function supports" as opposed to
> "you've passed a completely bogus clock", but I was worried that this would
> be a maintenance problem since it requires a definitive list of valid
> clocks and there would be a risk of getting the error wrong until someone
> noticed that the list hadn't been updated for a new clock. Of course, we
> could just return ENOTSUP for any unsupported clockid value which would
> avoid that problem.
> 
> It looks like it's the kernel that knows whether the clock is supported for
> clock_nanosleep and it falls out easily from the way the kernel implements
> clocks.
> 
> But, I'm not against improving the error as you describe if the glibc
> maintainers don't believe it will be a maintenance problem.

My understanding is to provided proper support for
pthread_mutex_clocklock/PI with CLOCK_MONOTONIC would require to extend
either current futex syscall or add it with the WIP futex2 one.

In either way, adding glibc support would require internal code changes
and we will need to revise and remove the error code.  So I don't see
a maintenance problem here.

> 
>> But I think it also depends whether either adding CLOCK_MONOTONIC
>> support for PI mutexes does make sense and if kernel would be willing
>> to add support for. It would probably require to add another flag
>> (FUTEX_CLOCK_MONOTONIC) to use along with FUTEX_LOCK_PI and I am not
>> sure who feasible it would be to implement within the kernel itself.
>> If any won't be considered, I think returning EINVAL does make sense.
> 
> I would hope that it would be possible to add support to the kernel for
> this at some point if it is found to be necessary.
> 
> Thanks.
> 
> Mike.
>