public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* What clocks are supported by pthread_clockjoin_np()
@ 2020-11-19  8:42 Michael Kerrisk (man-pages)
  2020-11-19 12:00 ` Mike Crowe
  0 siblings, 1 reply; 9+ messages in thread
From: Michael Kerrisk (man-pages) @ 2020-11-19  8:42 UTC (permalink / raw)
  To: Mike Crowe; +Cc: linux-man, libc-alpha

Hi Mike,

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?

Thanks,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: What clocks are supported by pthread_clockjoin_np()
  2020-11-19  8:42 What clocks are supported by pthread_clockjoin_np() Michael Kerrisk (man-pages)
@ 2020-11-19 12:00 ` Mike Crowe
  2020-11-19 12:29   ` Adhemerval Zanella
  0 siblings, 1 reply; 9+ messages in thread
From: Mike Crowe @ 2020-11-19 12:00 UTC (permalink / raw)
  To: Michael Kerrisk (man-pages); +Cc: linux-man, libc-alpha

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.

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

Thanks.

Mike.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: What clocks are supported by pthread_clockjoin_np()
  2020-11-19 12:00 ` Mike Crowe
@ 2020-11-19 12:29   ` Adhemerval Zanella
  2020-11-20  9:10     ` Mike Crowe
  0 siblings, 1 reply; 9+ messages in thread
From: Adhemerval Zanella @ 2020-11-19 12:29 UTC (permalink / raw)
  To: libc-alpha



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.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: What clocks are supported by pthread_clockjoin_np()
  2020-11-19 12:29   ` Adhemerval Zanella
@ 2020-11-20  9:10     ` Mike Crowe
  2020-11-20 12:40       ` Adhemerval Zanella
  0 siblings, 1 reply; 9+ messages in thread
From: Mike Crowe @ 2020-11-20  9:10 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha, Michael Kerrisk (man-pages)

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: What clocks are supported by pthread_clockjoin_np()
  2020-11-20  9:10     ` Mike Crowe
@ 2020-11-20 12:40       ` Adhemerval Zanella
  2020-11-20 14:22         ` Mike Crowe
  0 siblings, 1 reply; 9+ messages in thread
From: Adhemerval Zanella @ 2020-11-20 12:40 UTC (permalink / raw)
  To: Mike Crowe; +Cc: libc-alpha, Michael Kerrisk (man-pages)



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.

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. 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: What clocks are supported by pthread_clockjoin_np()
  2020-11-20 12:40       ` Adhemerval Zanella
@ 2020-11-20 14:22         ` Mike Crowe
  2020-11-20 20:27           ` Adhemerval Zanella
  0 siblings, 1 reply; 9+ messages in thread
From: Mike Crowe @ 2020-11-20 14:22 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha, Michael Kerrisk (man-pages)

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

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.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: What clocks are supported by pthread_clockjoin_np()
  2020-11-20 14:22         ` Mike Crowe
@ 2020-11-20 20:27           ` Adhemerval Zanella
  2020-11-20 21:48             ` Mike Crowe
  0 siblings, 1 reply; 9+ messages in thread
From: Adhemerval Zanella @ 2020-11-20 20:27 UTC (permalink / raw)
  To: Mike Crowe; +Cc: libc-alpha, Michael Kerrisk (man-pages)



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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: What clocks are supported by pthread_clockjoin_np()
  2020-11-20 20:27           ` Adhemerval Zanella
@ 2020-11-20 21:48             ` Mike Crowe
  2020-11-21  6:49               ` Michael Kerrisk (man-pages)
  0 siblings, 1 reply; 9+ messages in thread
From: Mike Crowe @ 2020-11-20 21:48 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha, Michael Kerrisk (man-pages)

On Friday 20 November 2020 at 17:27:03 -0300, Adhemerval Zanella wrote:
> On 20/11/2020 11:22, Mike Crowe wrote:
> > 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.

To distinguish between "valid clock IDs, but not ones that are supported by
the function" and "completely bogus clock IDs", I think that we'd need a
list of clocks. Something like:

if ((clockid & 7) == CLOCKFD)
  return ENOTSUP; // valid clockid, but not supported
switch (clockid) {
  case CLOCK_REALTIME:
  case CLOCK_MONOTONIC:
    // Those two clocks are supported, so we can continue.
    break;
  case CLOCK_REALTIME_COARSE:
  case CLOCK_TAI:
  case CLOCK_MONOTONIC_COARSE:
  case CLOCK_MONOTONIC_RAW:
  case CLOCK_BOOTTIME:
  case CLOCK_PROCESS_CPUTIME_ID:
  case ...possibly.some.I.missed...
    return ENOTSUP; // valid clockid, but not supported today
  case CLOCK_BOOTTIME_ALARM:
  case CLOCK_REALTIME_ALARM
  case CLOCK_THREAD_CPUTIME_ID:
    return EINVAL; // valid clockid, but can never be supported
  default:
    return EINVAL; // invalid clockid
}

(but in the pthread_mutex_clocklock/PI case CLOCK_MONOTONIC would return
ENOTSUP)

and someone would have to remember to add any new clocks that are invented
in the future. It's much easier to just have:

switch (clockid) {
  case CLOCK_REALTIME:
  case CLOCK_MONOTONIC:
    // Those two clocks are supported, so we can continue.
    break;
  default:
    return EINVAL or perhaps ENOTSUP;
}

which is effectively what we have at the moment in the
lll_futex_supported_clockid macro.

So, I believe we have three options:

1. Return ENOTSUP for valid-but-not-supported clocks and EINVAL for
   completely-invalid clocks. (i.e. the massive switch statement.)

2. Return EINVAL for any clockid values that are not supported by the
   code. (i.e. what we have now mostly)

3. Return ENOTSUP for any clockid values that are not supported by the
   code. (i.e. what we have now mostly, but change the error returned.)

The spec currently says 2, but I think it can be changed if there's a
consensus for a different option here.

Mike.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: What clocks are supported by pthread_clockjoin_np()
  2020-11-20 21:48             ` Mike Crowe
@ 2020-11-21  6:49               ` Michael Kerrisk (man-pages)
  0 siblings, 0 replies; 9+ messages in thread
From: Michael Kerrisk (man-pages) @ 2020-11-21  6:49 UTC (permalink / raw)
  To: Mike Crowe, Adhemerval Zanella; +Cc: mtk.manpages, libc-alpha

On 11/20/20 10:48 PM, Mike Crowe wrote:
> On Friday 20 November 2020 at 17:27:03 -0300, Adhemerval Zanella wrote:
>> On 20/11/2020 11:22, Mike Crowe wrote:
>>> 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.
> 
> To distinguish between "valid clock IDs, but not ones that are supported by
> the function" and "completely bogus clock IDs", I think that we'd need a
> list of clocks. Something like:
> 
> if ((clockid & 7) == CLOCKFD)
>   return ENOTSUP; // valid clockid, but not supported
> switch (clockid) {
>   case CLOCK_REALTIME:
>   case CLOCK_MONOTONIC:
>     // Those two clocks are supported, so we can continue.
>     break;
>   case CLOCK_REALTIME_COARSE:
>   case CLOCK_TAI:
>   case CLOCK_MONOTONIC_COARSE:
>   case CLOCK_MONOTONIC_RAW:
>   case CLOCK_BOOTTIME:
>   case CLOCK_PROCESS_CPUTIME_ID:
>   case ...possibly.some.I.missed...
>     return ENOTSUP; // valid clockid, but not supported today
>   case CLOCK_BOOTTIME_ALARM:
>   case CLOCK_REALTIME_ALARM
>   case CLOCK_THREAD_CPUTIME_ID:
>     return EINVAL; // valid clockid, but can never be supported
>   default:
>     return EINVAL; // invalid clockid
> }
> 
> (but in the pthread_mutex_clocklock/PI case CLOCK_MONOTONIC would return
> ENOTSUP)
> 
> and someone would have to remember to add any new clocks that are invented
> in the future. It's much easier to just have:
> 
> switch (clockid) {
>   case CLOCK_REALTIME:
>   case CLOCK_MONOTONIC:
>     // Those two clocks are supported, so we can continue.
>     break;
>   default:
>     return EINVAL or perhaps ENOTSUP;
> }
> 
> which is effectively what we have at the moment in the
> lll_futex_supported_clockid macro.
> 
> So, I believe we have three options:
> 
> 1. Return ENOTSUP for valid-but-not-supported clocks and EINVAL for
>    completely-invalid clocks. (i.e. the massive switch statement.)
> 
> 2. Return EINVAL for any clockid values that are not supported by the
>    code. (i.e. what we have now mostly)
> 
> 3. Return ENOTSUP for any clockid values that are not supported by the
>    code. (i.e. what we have now mostly, but change the error returned.)
> 
> The spec currently says 2, but I think it can be changed if there's a
> consensus for a different option here.

Looking at some existing specifications in POSIX...

    pthread_condattr_getclock(3p:

       The pthread_condattr_setclock() function may fail if:

       EINVAL The value specified by clock_id does not refer to  a  known
              clock, or is a CPU-time clock.

    clock_getres(3p):

       The clock_getres(), clock_gettime(), and clock_settime() functions
       shall fail if:

       EINVAL The clock_id argument does not specify a known clock.

This suggests that option 2 is the way to go.

Thanks,

Michael

-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-11-21  6:49 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-19  8:42 What clocks are supported by pthread_clockjoin_np() Michael Kerrisk (man-pages)
2020-11-19 12:00 ` Mike Crowe
2020-11-19 12:29   ` Adhemerval Zanella
2020-11-20  9:10     ` Mike Crowe
2020-11-20 12:40       ` Adhemerval Zanella
2020-11-20 14:22         ` Mike Crowe
2020-11-20 20:27           ` Adhemerval Zanella
2020-11-20 21:48             ` Mike Crowe
2020-11-21  6:49               ` Michael Kerrisk (man-pages)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).