This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 2/6] linux: Assume clock_getres CLOCK_{PROCESS,THREAD}_CPUTIME_ID
On 12/02/2019 11:17, Florian Weimer wrote:
> * Adhemerval Zanella:
>
>> On 11/02/2019 11:22, Carlos O'Donell wrote:
>>> On 2/11/19 7:54 AM, Florian Weimer wrote:
>>>> * Adhemerval Zanella:
>>>>
>>>>> On 11/02/2019 09:43, Florian Weimer wrote:
>>>>>> * Adhemerval Zanella:
>>>>>>
>>>>>>> nscd uses _SC_MONOTONIC_CLOCK and if I understood correctly both IA64
>>>>>>> manual and kernel code, the CLOCK_MONOTONIC implementation on SMP IA64
>>>>>>> system with ITC that may drift is a best effort...
>>>>>>
>>>>>> Are you sure about that? Wouldn't that be a bug in kernel
>>>>>> clock_gettime, and not something we should paper over in userspace?
>>>>>
>>>>> Not sure, I was just my impression checking kernel ia64 code but I am
>>>>> not an architecture expert. To be safe I kept the original code in
>>>>> patch proposal.
>>>>
>>>> I think it's not worth maintaing the ia64 divergence for this. If it is
>>>> a problem with the kernel's CLOCK_MONOTONIC implementation, it has to be
>>>> fixed there.
>>>
>>> I tend to agree. Standard interfaces should behave in standard ways.
>>>
>>> We have the same problem in HPPA where a similar clock (and my feeling was
>>> always that IA64 derived a lot from HP PARISC 64-bit) that needs synchronization
>>> at SMP rendevous at boot time, followed by bias and drift adjustment.
>>>
>>> For HPPA we made sue that the everything was accurately compensated at the
>>> kernel timer source level, and never in userspace. You *can* do it in userspace
>>> and it's faster and lower latency, and all of those things, but it's more complex
>>> and prone to breakage for odd reasons (compiler optimizations etc.).
>>>
>>> For HPPA you can access the tick counter in userspace, raw, and it's
>>> useful for timing small sequences of code, but if you move to a new CPU
>>> mid-measurement, you'll get wrong values because the value and bias is per-cpu.
>>>
>>
>> Simplified patch below. I decided to keep the syscall issue so kernel can
>> correctly report its supports.
>>
>> * sysdeps/unix/sysv/linux/ia64/has_cpuclock.c: Remove file.
>> * sysdeps/unix/sysv/linux/ia64/sysconf.c: Likewise.
>> * sysdeps/unix/sysv/linux/sysconf.c (has_cpuclock): Remove function.
>> (check_clock_getres): New function.
>> (__sysconf): Use check_clock_getres instead of has_cpuclock.
>
> __sysconf now has:
>
> case _SC_MONOTONIC_CLOCK:
> return check_clock_getres (CLOCK_MONOTONIC);
>
> case _SC_CPUTIME:
> return check_clock_getres (CLOCK_PROCESS_CPUTIME_ID);
> case _SC_THREAD_CPUTIME:
> return check_clock_getres (CLOCK_THREAD_CPUTIME_ID);
>
> I suggest to remove that blank line.
Ack, I changed it locally.
>
> It may make sense to add a note to check_clock_getres that we use
> INTERNAL_SYSCALL_CALL to avoid setting errno, so that the call can use
> errno to check whether sysconf recognized the parameter constant at all.
Ack, I added:
/* Avoid setting errno so it can check whether the kernel supports
the CLK_ID. */
As a side note I think we can also use INLINE_SYCALL_CALL and check for EINVAL.
>
> Otherwise, looks good to me.
>
> Thanks,
> Florian
>