Project

General

Profile

Actions

Bug #14126

closed

clock_gettime() could work with thread/proc clocks

Added by Tim Mooney 2 months ago. Updated about 1 month ago.

Status:
Closed
Priority:
Low
Category:
kernel
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

Description

Hi All-

Illumos system headers provide the CLOCK_THREAD_CPUTIME_ID define, and other operating systems (including Oracle Solaris 11.4 now, but not earlier versions) use that with clock_gettime() to do high resolution timing in thread.

The issue is that the implementation we inherited from Solaris is either incomplete or intentionally non-functional. Calling clock_gettime() with CLOCK_THREAD_CPUTIME_ID always results in EINVAL error return.

A couple of open source projects (first Python, now GnuPG) have had to develop a workaround for the present-but-broken CLOCK_THREAD_CPUTIME_ID.

This is an area where recent Oracle Solaris and Illumos have now diverged. The 11.4 kernel has fixed the issue, but older Oracle Solaris and Illumos have the non-working implementation. This makes it a little more difficult for open source developers, because they can't make a decision about whether to use CLOCK_THREAD_CPUTIME_ID based on something like #if defined(HAVE_CLOCK_THREAD_CPUTIME_ID) && !defined(__sun). Even when they are willing to implement workarounds using something like gethrvtime(), it's difficult for them to programmatically detect when the workaround is needed.

The Python bug:

https://bugs.python.org/issue35455

The GnuPG bug (references the Python bug):

https://dev.gnupg.org/T5623

Discussion about the issue on the OI mailing list (starting mid-thread):

https://openindiana.org/pipermail/openindiana-discuss/2021-September/025070.html

Files

gnupg-2.3.2_testsuite.log (4.11 KB) gnupg-2.3.2_testsuite.log Dominik Hassler, 2021-10-05 06:23 PM

Related issues

Has duplicate illumos gate - Feature #12773: support CLOCK_THREAD_CPUTIME_ID and CLOCK_PROCESS_CPUTIME_IDClosed

Actions
Blocks illumos gate - Feature #14140: timer_create()/timer_settime() could work with thread/proc clocksNew

Actions
Actions #1

Updated by Robert Mustacchi about 2 months ago

Thanks for taking the time to write this up. I have a couple of questions:

  • I presume folks don't expect to be able to use this with clock_settime(), that is that can sitll fail.
  • Do you know if folks expect to be able to create timers based around this?

I'm trying to figure out if the better coarse of action in the short term is to make clock_gettime() work for this clock and the process clock or if it's better to actually just remove it from the header file.

Actions #2

Updated by Robert Mustacchi about 2 months ago

I took a bit more of a look at this in general. Based on this I learned the following:
  • Most folks don't support clock_settime() on these clocks
  • timer_create() support is mixed:
    • Linux has support for all of these.
    • FreeBSD only supports the HIGHRES and REALTIME clocks
    • NetBSD supports it for the REALTIME, MONOTONIC, VIRTUAL, and PROF clocks, but not if you use CLOCK_THREAD_CPUTIME_ID or CLOCK_PROCESS_CPUTIME_ID
    • OpenBSD and DragonFly don't support the interface at all

Based on this, I've taken a stab at implementing the clock backends with clock_gettime and clock_getres for the following clocks:

  • CLOCK_VIRTUAL
  • CLOCK_THREAD_CPUTIME_ID
  • CLOCK_PROCESS_CPUTIME_ID

Tim, would you be in a place where you could test this against python and/or GnuPG to make sure that things are working correctly for them with this implemented?

Actions #3

Updated by Robert Mustacchi about 2 months ago

  • Category set to kernel
  • Assignee set to Robert Mustacchi
  • % Done changed from 0 to 60
Actions #4

Updated by Tim Mooney about 2 months ago

Tim, would you be in a place where you could test this against python and/or GnuPG to make sure that things are working correctly for them with this implemented?

Thanks Robert, and sorry I didn't get a response in to your question before now.

I can test with GnuPG, but it will probably be a day or two. I need to check with Andreas W. and possibly others to see when this is in OI's kernel. I'll update and test after OI has it.

I really appreciate your time on this. No pun intended. :-)

Thanks,

Tim

Actions #5

Updated by Robert Mustacchi about 2 months ago

  • Subject changed from clock_gettime() with CLOCK_THREAD_CPUTIME_ID always returns EINVAL to clock_gettime() could work with thread/proc clocks
Actions #6

Updated by Tim Mooney about 2 months ago

Tim, would you be in a place where you could test this against python and/or GnuPG to make sure that things are working correctly for them with this implemented?

Hi Robert, I'm afraid I'm not experienced enough with illumos-gate to know how to find your kernel code changes for this.

Can you point me to a diff that has the relevant code?

I have a set up oi-userland environment for building and testing and Andreas Wacknitz has provided an overview of what I need to do for illumos-gate patching and testing.

Sorry for the newb question, I've just never needed to touch illumos-gate before.

Tim

Actions #7

Updated by Robert Mustacchi about 2 months ago

  • Gerrit CR set to 1740
Actions #8

Updated by Tim Mooney about 2 months ago

Thanks Robert.

I have the patch, applied cleanly to master and building illumos-gate now. Assuming no build issues, I should be able to test tomorrow.

Actions #9

Updated by Tim Mooney about 2 months ago

Hi Robert-

The GnuPG developers just mentioned something else about clock_gettime(): the OpenGroup standards and _POSIX_THREAD_CPUTIME and _POSIX_CPUTIME.

See:

https://pubs.opengroup.org/onlinepubs/9699919799/functions/clock_gettime.html
https://docs.oracle.com/cd/E36784_01/html/E36873/unistd.h-3head.html

It seems likely your implementation should now also be setting _POSIX_THREAD_CPUTIME and possibly _POSIX_CPUTIME .

Thanks

Actions #10

Updated by Robert Mustacchi about 2 months ago

We can't set those yet as we don't currently support those clocks, which are a way of getting an arbitrary process or thread's time as opposed to the caller. I did not add support for them here. Does gpg require them?

Actions #11

Updated by Dominik Hassler about 2 months ago

I've pulled the proposed patch on top of omnios bloody and ran the test-suite for gnupg 2.3.2. The test-suite (results attached) used to lock-up before but now runs successfully.

Actions #12

Updated by Dan McDonald about 2 months ago

  • Blocks Feature #14140: timer_create()/timer_settime() could work with thread/proc clocks added
Actions #13

Updated by Andy Fiddaman about 2 months ago

  • Has duplicate Feature #12773: support CLOCK_THREAD_CPUTIME_ID and CLOCK_PROCESS_CPUTIME_ID added
Actions #14

Updated by Robert Mustacchi about 2 months ago

In addition to the testing that Dominik did, I wrote a test suite for this. I ran it as part of the os-tests. There is one test which failed and that is due to an issue with how that test is written (and fails irrespective of this change). Here's the test run:

rm@beowulf:~$ pfexec /opt/os-tests/bin/ostest 
Test: /opt/os-tests/tests/clock_gettime.32 (run as root)          [00:00] [PASS]
Test: /opt/os-tests/tests/clock_gettime.64 (run as root)          [00:00] [PASS]
Test: /opt/os-tests/tests/eventfd.32 (run as root)                [00:00] [PASS]
Test: /opt/os-tests/tests/eventfd.64 (run as root)                [00:00] [PASS]
Test: /opt/os-tests/tests/imc_test (run as root)                  [00:00] [PASS]
Test: /opt/os-tests/tests/odirectory.32 (run as root)             [00:00] [PASS]
Test: /opt/os-tests/tests/odirectory.64 (run as root)             [00:00] [PASS]
Test: /opt/os-tests/tests/writev.32 (run as root)                 [00:00] [PASS]
Test: /opt/os-tests/tests/writev.64 (run as root)                 [00:00] [PASS]
Test: /opt/os-tests/tests/ddi_ufm/ufm-test-setup (run as root)    [00:00] [PASS]
Test: /opt/os-tests/tests/ddi_ufm/ufm-test (run as root)          [00:00] [PASS]
Test: /opt/os-tests/tests/ddi_ufm/ufm-test-cleanup (run as root)  [00:00] [PASS]
Test: /opt/os-tests/tests/file-locking/runtests.32 (run as root)  [00:21] [PASS]
Test: /opt/os-tests/tests/file-locking/runtests.64 (run as root)  [00:21] [PASS]
Test: /opt/os-tests/tests/i386/ldt (run as root)                  [00:06] [PASS]
Test: /opt/os-tests/tests/i386/badseg (run as root)               [00:05] [PASS]
Test: /opt/os-tests/tests/ksensor/ksensor_init (run as root)      [00:00] [PASS]
Test: /opt/os-tests/tests/ksensor/ksensor_basic.32 (run as root)  [00:00] [PASS]
Test: /opt/os-tests/tests/ksensor/ksensor_basic.64 (run as root)  [00:00] [PASS]
Test: /opt/os-tests/tests/ksensor/ksensor_err.32 (run as root)    [00:00] [PASS]
Test: /opt/os-tests/tests/ksensor/ksensor_err.64 (run as root)    [00:00] [PASS]
Test: /opt/os-tests/tests/ksensor/ksensor_fini (run as root)      [00:00] [PASS]
Test: /opt/os-tests/tests/libtopo/digraph-test (run as root)      [00:00] [PASS]
Test: /opt/os-tests/tests/pf_key/acquire-compare (run as root)    [00:43] [PASS]
Test: /opt/os-tests/tests/pf_key/kmc-update (run as root)         [00:00] [PASS]
Test: /opt/os-tests/tests/sdevfs/sdevfs_eisdir (run as root)      [00:00] [PASS]
Test: /opt/os-tests/tests/secflags/secflags_aslr (run as root)    [00:02] [PASS]
Test: /opt/os-tests/tests/secflags/secflags_core (run as root)    [00:01] [PASS]
Test: /opt/os-tests/tests/secflags/secflags_dts (run as root)     [00:02] [FAIL]
Test: /opt/os-tests/tests/secflags/secflags_elfdump (run as root) [00:01] [PASS]
Test: /opt/os-tests/tests/secflags/secflags_forbidnullmap (run as root) [00:00] [PASS]
Test: /opt/os-tests/tests/secflags/secflags_limits (run as root)  [00:00] [PASS]
Test: /opt/os-tests/tests/secflags/secflags_noexecstack (run as root) [00:00] [PASS]
Test: /opt/os-tests/tests/secflags/secflags_proc (run as root)    [00:01] [PASS]
Test: /opt/os-tests/tests/secflags/secflags_psecflags (run as root) [00:11] [PASS]
Test: /opt/os-tests/tests/secflags/secflags_syscall (run as root) [00:00] [PASS]
Test: /opt/os-tests/tests/secflags/secflags_truss (run as root)   [00:00] [PASS]
Test: /opt/os-tests/tests/secflags/secflags_zonecfg (run as root) [00:00] [PASS]
Test: /opt/os-tests/tests/sigqueue/sigqueue_queue_size (run as root) [00:00] [PASS]
Test: /opt/os-tests/tests/sockfs/conn (run as root)               [00:05] [PASS]
Test: /opt/os-tests/tests/sockfs/dgram (run as root)              [00:05] [PASS]
Test: /opt/os-tests/tests/sockfs/drop_priv (run as root)          [00:07] [PASS]
Test: /opt/os-tests/tests/sockfs/nosignal (run as root)           [00:02] [PASS]
Test: /opt/os-tests/tests/sockfs/rights.32 (run as root)          [00:00] [PASS]
Test: /opt/os-tests/tests/sockfs/rights.64 (run as root)          [00:00] [PASS]
Test: /opt/os-tests/tests/sockfs/sockpair (run as root)           [00:10] [PASS]
Test: /opt/os-tests/tests/sockfs/recvmsg.32 (run as root)         [00:00] [PASS]
Test: /opt/os-tests/tests/sockfs/recvmsg.64 (run as root)         [00:00] [PASS]
Test: /opt/os-tests/tests/stackalign/stackalign.32 (run as root)  [00:00] [PASS]
Test: /opt/os-tests/tests/stackalign/stackalign.64 (run as root)  [00:00] [PASS]
Test: /opt/os-tests/tests/stress/dladm-kstat (run as root)        [00:20] [PASS]
Test: /opt/os-tests/tests/syscall/open.32 (run as root)           [00:00] [PASS]
Test: /opt/os-tests/tests/syscall/open.64 (run as root)           [00:00] [PASS]
Test: /opt/os-tests/tests/timer/timer_limit (run as root)         [00:00] [PASS]
Test: /opt/os-tests/tests/uccid/atrparse (run as root)            [00:00] [PASS]

Results Summary
FAIL       1
PASS      54

Running Time:    00:02:49
Percent passed:    98.2%
Log directory:    /var/tmp/test_results/20211015T132524
Actions #15

Updated by Electric Monk about 1 month ago

  • Status changed from New to Closed
  • % Done changed from 60 to 100

git commit dea9f5e6a4938723acec9624b3aa3f680f2f5c9f

commit  dea9f5e6a4938723acec9624b3aa3f680f2f5c9f
Author: Robert Mustacchi <rm@fingolfin.org>
Date:   2021-10-16T14:56:11.000Z

    14126 clock_gettime() could work with thread/proc clocks
    14139 Want libproc convenience function for lwpsinfo
    Reviewed by: C Fraire <cfraire@me.com>
    Reviewed by: Andy Fiddaman <andy@omnios.org>
    Reviewed by: Toomas Soome <tsoome@me.com>
    Approved by: Dan McDonald <danmcd@joyent.com>

Actions

Also available in: Atom PDF