What clocks are supported by pthread_clockjoin_np()

[Mike Crowe]

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.