posix + tests: correct timing functions to use CLOCK_REALTIME #88762

cfriedt · 2025-04-17T11:56:42Z

Use CLOCK_REALTIME by default as per the POSIX specification for the following functions:

mq_timedreceive()
mq_timedsend()
pthread_cond_timedwait()
pthread_mutex_timedlock()
pthread_rwlock_timedrdlock()
pthread_rwlock_timedwrlock()
sem_timedwait()

and additionally for the following non-portable functions:

pthread_timedjoin_np()

Should be merged after #88767 and #88769

cfriedt · 2025-04-22T14:47:43Z

An aside, I think it would be really useful to have an API call to perform common operations on timespec objects, sort of like https://github.com/solemnwarning/timespec, that additionally reports when an overflow has occurred.

That API and function signatures are good, but I would almost prefer if the entire implementation were in a header file, as static inlines, and additionally be able to use __builtin_add_overflow() and friends, as long as C11 was enabled (and the builtin was supported by the toolchain).

https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Builtins.html

Many of the algorithms can also be branchless as well, which will speed things up on processors that use branch prediction. Additionally, we would want to move many runtime checks to assertions so that they can be disabled for production firmware.

Lots of small optimizations for bare metal.

And of course, struct timespec is not specific to POSIX, as it's been part of ISO C since C11.

Add definitions for struct posix_condattr and struct posix_cond, which are internal variants of the external pthread_condattr_t and pthread_cond_t types. Signed-off-by: Chris Friedt <[email protected]>

cfriedt · 2025-04-23T13:50:50Z

reworked PR into smaller logical commits
dropped some changes not relevant here (yet) related to the POSIX_CLOCK_SELECTION Option Group

Use struct posix_cond and struct posix_condattr internally. Signed-off-by: Chris Friedt <[email protected]>

Check whenther a pthread_condattr_t has been initialized in pthread_condattr_*() functions. Signed-off-by: Chris Friedt <[email protected]>

Use the clock specified via pthread_condattr_t in pthread_cond_timedwait(). Signed-off-by: Chris Friedt <[email protected]>

Add a common private function timespec_is_valid() that can be used to check if a timespec object is valid, and use that consistently in lib/posix/options. Signed-off-by: Chris Friedt <[email protected]>

Use CLOCK_REALTIME for the default clock source throughout the POSIX implementation and tests so that we are consistent with the specification. Signed-off-by: Chris Friedt <[email protected]>

Always require the clockid_t argument to timespec_to_timeoutms() and remove the unused variant that accepts no clockid_t parameter. Signed-off-by: Chris Friedt <[email protected]>

Prevent doxygen from parsing internal functions declared in posix_clock.h . Signed-off-by: Chris Friedt <[email protected]>

cfriedt · 2025-04-23T14:43:28Z

removed clever macro-based timespec functions due to compliance warnings 🫠

tests/posix/timers/src/clock.c

Move somewhat useful (but private and internal functions) that deal with struct timespec to posix_clock.h until there is a better API available for dealing with operations on struct timespec. Signed-off-by: Chris Friedt <[email protected]>

Use the tp_diff() macro as a means of converting an absolute timeout with respect to a specific clock to a relative timeout, in ms. Clamp the result between 0 and UINT32_MAX. Signed-off-by: Chris Friedt <[email protected]>

cfriedt · 2025-04-23T18:06:59Z

replaced tp_ge() in posix_clock.h

Copilot

Pull Request Overview

This pull request updates various POSIX timing functions and their corresponding tests to use CLOCK_REALTIME by default, in line with the POSIX specification. Key changes include:

Replacing CLOCK_MONOTONIC with CLOCK_REALTIME in time-related functions and tests.
Refactoring timeout conversion functions to use the new posix_clock.h utilities.
Standardizing error checking for invalid timespec objects across multiple modules.

Reviewed Changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/posix/timers/src/clock.c	Removed custom inline time conversion functions in favor of posix_clock.h helpers.
tests/posix/rwlocks/src/main.c	Updated timeout setup to use clock_gettime(CLOCK_REALTIME, …).
tests/posix/common/src/pthread.c	Adjusted error checking in pthread_timedjoin_np for timespec validation.
tests/posix/common/src/cond.c	Updated clock attribute expectations from CLOCK_MONOTONIC to CLOCK_REALTIME.
tests/lib/c_lib/thrd/src/cnd.c	Modified timedwait to use CLOCK_REALTIME instead of CLOCK_MONOTONIC.
lib/posix/options/timespec_to_timeout.c	Updated function signature to include clock_id and use CLAMP with tp_diff.
lib/posix/options/timer.c	Replaced legacy timeout conversion macros with ts_to_ms and validated timespecs.
lib/posix/options/semaphore.c	Updated sem_timedwait to use the new timespec_to_timeoutms conversion.
lib/posix/options/rwlock.c	Modified read/write lock acquire functions to use uint32_t timeouts.
lib/posix/options/pthread.c	Adjusted pthread_timedjoin_np to validate the abstime using the new utility.
lib/posix/options/posix_internal.h	Introduced new posix_condattr structure for condition variable attributes.
lib/posix/options/posix_clock.h	Added common timespec utility functions (e.g. timespec_is_valid, ts_to_ns).
lib/posix/options/mutex.c	Updated pthread_mutex_timedlock to check timespec validity and use CLOCK_REALTIME.
lib/posix/options/mqueue.c	Changed both mq_timedsend and mq_timedreceive to convert times using CLOCK_REALTIME.
lib/posix/options/cond.c	Refactored condition variable functions to use posix_cond structures and updated attribute handling.
include/zephyr/posix/time.h	Removed unused inline timeout conversion functions.

Files not reviewed (1)

tests/posix/timers/CMakeLists.txt: Language not supported

Copilot · 2025-04-25T09:23:58Z

lib/posix/options/cond.c

 }

 int pthread_cond_init(pthread_cond_t *cvar, const pthread_condattr_t *att)
 {
-	struct k_condvar *cv;
+	struct posix_cond *cv;
+	struct posix_condattr *attr = (struct posix_condattr *)attr;


The local variable 'attr' is incorrectly initialized by casting itself instead of the function parameter. Change it to 'struct posix_condattr *attr = (struct posix_condattr *)att;' to correctly reference the passed attribute.

Suggested change

struct posix_condattr *attr = (struct posix_condattr *)attr;

struct posix_condattr *attr = (struct posix_condattr *)att;

cfriedt force-pushed the issue/88587/posix-pthread-condition-variables-always-wait-on-clock-monotonic branch 2 times, most recently from f35cc31 to f3c0a2e Compare April 17, 2025 12:41

cfriedt changed the title ~~posix + tests: correct a number of timing functions to use CLOCK_REAL…~~ posix + tests: correct timing functions to use CLOCK_REALTIME Apr 17, 2025

cfriedt force-pushed the issue/88587/posix-pthread-condition-variables-always-wait-on-clock-monotonic branch 14 times, most recently from 95b410c to 4def3d8 Compare April 18, 2025 12:52

cfriedt mentioned this pull request Apr 18, 2025

posix: options: add custom Zephyr POSIX subprofile #88547

Draft

cfriedt force-pushed the issue/88587/posix-pthread-condition-variables-always-wait-on-clock-monotonic branch 7 times, most recently from 9bd13da to af01824 Compare April 22, 2025 14:42

cfriedt marked this pull request as ready for review April 22, 2025 15:33

github-actions bot added area: C Library C Standard Library area: POSIX POSIX API Library labels Apr 22, 2025

github-actions bot requested a review from aescolar April 22, 2025 15:34

posix: posix_internal: add definitions for posix_condattr, posix_cond

a73da15

Add definitions for struct posix_condattr and struct posix_cond, which are internal variants of the external pthread_condattr_t and pthread_cond_t types. Signed-off-by: Chris Friedt <[email protected]>

cfriedt dismissed ycsin’s stale review via 27bb2cf April 23, 2025 13:49

cfriedt force-pushed the issue/88587/posix-pthread-condition-variables-always-wait-on-clock-monotonic branch from af01824 to 27bb2cf Compare April 23, 2025 13:49

cfriedt force-pushed the issue/88587/posix-pthread-condition-variables-always-wait-on-clock-monotonic branch 4 times, most recently from 6fa3884 to 21aaee9 Compare April 23, 2025 14:16

cfriedt added 7 commits April 23, 2025 10:21

posix: cond: use struct posix_cond and struct posix_condattr internally

6877c18

Use struct posix_cond and struct posix_condattr internally. Signed-off-by: Chris Friedt <[email protected]>

posix: cond: check whether condattr is initialized in attr fns

3f8da89

Check whenther a pthread_condattr_t has been initialized in pthread_condattr_*() functions. Signed-off-by: Chris Friedt <[email protected]>

posix: cond: use clock specified via pthread_condattr_t

bead984

Use the clock specified via pthread_condattr_t in pthread_cond_timedwait(). Signed-off-by: Chris Friedt <[email protected]>

posix: add timespec_is_valid() private internal function

ce4942b

Add a common private function timespec_is_valid() that can be used to check if a timespec object is valid, and use that consistently in lib/posix/options. Signed-off-by: Chris Friedt <[email protected]>

posix + tests: use CLOCK_REALTIME where specified by POSIX

9ff91f1

Use CLOCK_REALTIME for the default clock source throughout the POSIX implementation and tests so that we are consistent with the specification. Signed-off-by: Chris Friedt <[email protected]>

posix: always require clockid_t argument to timespec_to_timeoutms()

c747880

Always require the clockid_t argument to timespec_to_timeoutms() and remove the unused variant that accepts no clockid_t parameter. Signed-off-by: Chris Friedt <[email protected]>

posix: doc: hide internal posix_clock.h functions from doxygen

5823344

Prevent doxygen from parsing internal functions declared in posix_clock.h . Signed-off-by: Chris Friedt <[email protected]>

cfriedt force-pushed the issue/88587/posix-pthread-condition-variables-always-wait-on-clock-monotonic branch from 21aaee9 to ee68f25 Compare April 23, 2025 14:42

cfriedt commented Apr 23, 2025

View reviewed changes

tests/posix/timers/src/clock.c Show resolved Hide resolved

cfriedt requested review from jukkar and ycsin April 23, 2025 14:48

cfriedt added 2 commits April 23, 2025 14:06

posix: refactor timespec_to_timeoutms() to use tp_diff()

9aff67e

Use the tp_diff() macro as a means of converting an absolute timeout with respect to a specific clock to a relative timeout, in ms. Clamp the result between 0 and UINT32_MAX. Signed-off-by: Chris Friedt <[email protected]>

cfriedt force-pushed the issue/88587/posix-pthread-condition-variables-always-wait-on-clock-monotonic branch from ee68f25 to 9aff67e Compare April 23, 2025 18:06

ycsin approved these changes Apr 24, 2025

View reviewed changes

jukkar approved these changes Apr 24, 2025

View reviewed changes

kartben merged commit e71c12c into zephyrproject-rtos:main Apr 24, 2025
24 checks passed

cfriedt deleted the issue/88587/posix-pthread-condition-variables-always-wait-on-clock-monotonic branch April 24, 2025 21:04

kartben requested a review from Copilot April 25, 2025 09:23

Copilot AI reviewed Apr 25, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

posix + tests: correct timing functions to use CLOCK_REALTIME #88762

posix + tests: correct timing functions to use CLOCK_REALTIME #88762

cfriedt commented Apr 17, 2025 •

edited

Loading

cfriedt commented Apr 22, 2025 •

edited

Loading

cfriedt commented Apr 23, 2025

cfriedt commented Apr 23, 2025

cfriedt commented Apr 23, 2025

Copilot AI left a comment

Copilot AI Apr 25, 2025

kartben Apr 25, 2025

	struct posix_condattr attr = (struct posix_condattr )attr;
	struct posix_condattr attr = (struct posix_condattr )att;

posix + tests: correct timing functions to use CLOCK_REALTIME #88762

posix + tests: correct timing functions to use CLOCK_REALTIME #88762

Conversation

cfriedt commented Apr 17, 2025 • edited Loading

cfriedt commented Apr 22, 2025 • edited Loading

cfriedt commented Apr 23, 2025

cfriedt commented Apr 23, 2025

cfriedt commented Apr 23, 2025

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Copilot AI Apr 25, 2025

Choose a reason for hiding this comment

kartben Apr 25, 2025

Choose a reason for hiding this comment

cfriedt commented Apr 17, 2025 •

edited

Loading

cfriedt commented Apr 22, 2025 •

edited

Loading