posix: Map CLOCK_REALTIME and CLOCK_MONOTONIC to Zephyr clock IDs #93075
Conversation
|
@M-Moawad - it looks like your commit message needs to be reformatted. https://github.com/zephyrproject-rtos/zephyr/actions/runs/16251745415/job/45886336337#step:10:1 Use |
M-Moawad
force-pushed
the
moawad-posix-align-clock-ids
branch
from
396fc20 to
670b68f
Compare
Done |
|
I'm not sure this will work long term because if you have a toolchain with a C library built against the old values, and then you redefine them to the Zephyr ones, you will have hard-to-debug issues calling clock_gettime and other functions from the C library dependent of whether you have this header included or not. Although AFAIU this will also depend on whether TC_PROVIDES_POSIX_TIMERS is set for the toolchain. @cfriedt, what do you think, where all of this should be going? |
That occurred to me as well. I think ideally, we would use the values defined by the C library and have a internal The I'm currently working on a change to remove those two from POSIX and consolidate them (most likely) into the minimal libc include directory. Actually, a lot of POSIX header cleanup in general. I think starting with One caveat for doing that (which is not so bad) is that libc's like IAR, which have no POSIX support (fwir), would need to either reuse definitions from the minimal libc or duplicate them within their own include directory (or maybe use common includes?) Another caveat for doing that, would be that we would need to define feature test macros likely via cmake. So for example The main benefit overall is that there would be fewer conflicts between libc headers and posix headers, and POSIX headers in Zephyr could be used mainly when the C library does not provide their own conformant headers. I think most people would prefer that. |
|
@M-Moawad @tagunil - can you check if this works with ARC MWDT? It might be the least amount of work in this case. diff --git a/include/zephyr/sys/clock.h b/include/zephyr/sys/clock.h
index 47fbfa3cf47..00e0a9a1ade 100644
--- a/include/zephyr/sys/clock.h
+++ b/include/zephyr/sys/clock.h
@@ -378,6 +378,9 @@ static inline bool sys_timepoint_expired(k_timepoint_t timepoint)
/** @cond INTERNAL_HIDDEN */
/* forward declaration as workaround for time.h */
struct timespec;
+
+/* Convert a POSIX clock (cast to int) to a sys_clock identifier */
+int sys_clock_from_clockid(int clock_id);
/** @endcond INTERNAL_HIDDEN */
/**
diff --git a/lib/os/clock.c b/lib/os/clock.c
index 98867ea3675..b34407da722 100644
--- a/lib/os/clock.c
+++ b/lib/os/clock.c
@@ -50,6 +50,22 @@ static void timespec_from_ticks(uint64_t ticks, struct timespec *ts)
};
}
+int sys_clock_from_clockid(int clock_id)
+{
+ switch (clock_id) {
+#if defined(CLOCK_REALTIME) || defined(_POSIX_C_SOURCE)
+ case (int)CLOCK_REALTIME:
+ return SYS_CLOCK_REALTIME;
+#endif
+#if defined(CLOCK_MONOTONIC) || defined(_POSIX_MONOTONIC_CLOCK)
+ case (int)CLOCK_MONOTONIC:
+ return SYS_CLOCK_MONOTONIC;
+#endif
+ default:
+ return -EINVAL;
+ }
+}
+
int sys_clock_gettime(int clock_id, struct timespec *ts)
{
if (!is_valid_clock_id(clock_id)) {
diff --git a/lib/posix/options/clock.c b/lib/posix/options/clock.c
index 239dadb6926..466974705ac 100644
--- a/lib/posix/options/clock.c
+++ b/lib/posix/options/clock.c
@@ -18,7 +18,7 @@ int clock_gettime(clockid_t clock_id, struct timespec *ts)
{
int ret;
- ret = sys_clock_gettime((int)clock_id, ts);
+ ret = sys_clock_gettime(sys_clock_from_clockid((int)clock_id), ts);
if (ret < 0) {
errno = -ret;
return -1;
@@ -61,7 +61,7 @@ int clock_settime(clockid_t clock_id, const struct timespec *tp)
{
int ret;
- ret = sys_clock_settime((int)clock_id, tp);
+ ret = sys_clock_settime(sys_clock_from_clockid((int)clock_id), tp);
if (ret < 0) {
errno = -ret;
return -1;
diff --git a/lib/posix/options/clock_selection.c b/lib/posix/options/clock_selection.c
index c67d48ff025..5717221b596 100644
--- a/lib/posix/options/clock_selection.c
+++ b/lib/posix/options/clock_selection.c
@@ -26,7 +26,7 @@ int clock_nanosleep(clockid_t clock_id, int flags, const struct timespec *rqtp,
return -1;
}
- ret = sys_clock_nanosleep((int)clock_id, flags, rqtp, rmtp);
+ ret = sys_clock_nanosleep(sys_clock_from_clockid((int)clock_id), flags, rqtp, rmtp);
if (ret < 0) {
errno = -ret;
return -1;
diff --git a/lib/posix/options/timespec_to_timeout.c b/lib/posix/options/timespec_to_timeout.c
index a0b3477616e..f8719a77eeb 100644
--- a/lib/posix/options/timespec_to_timeout.c
+++ b/lib/posix/options/timespec_to_timeout.c
@@ -17,7 +17,7 @@ uint32_t timespec_to_timeoutms(int clock_id, const struct timespec *abstime)
{
struct timespec curtime;
- if (sys_clock_gettime(clock_id, &curtime) < 0) {
+ if (sys_clock_gettime(sys_clock_from_clockid(clock_id), &curtime) < 0) {
return 0;
}
I think that should handle all use cases, but please correct me if there are any call sites missing. $ twister --retry-failed 3 -T tests/posix/timers/
...
INFO - Total complete: 40/ 40 100% built (not run): 0, filtered: 145, failed: 0, error: 0
INFO - 4 test scenarios (184 configurations) selected, 145 configurations filtered (144 by static filter, 1 at runtime).
INFO - 39 of 39 executed test configurations passed (100.00%), 0 built (not run), 0 failed, 0 errored, with no warnings in 164.44 seconds.
INFO - 624 of 624 executed test cases passed (100.00%) on 10 out of total 1115 platforms (0.90%).
INFO - 39 test configurations executed on platforms, 0 test configurations were only built.
INFO - Saving reports...
INFO - Writing JSON report /home/cfriedt/zephyrproject/zephyr/twister-out/twister.json
INFO - Writing xunit report /home/cfriedt/zephyrproject/zephyr/twister-out/twister.xml...
INFO - Writing xunit report /home/cfriedt/zephyrproject/zephyr/twister-out/twister_report.xml...
INFO - Run completed |
There was a problem hiding this comment.
As mentioned in the linked comment, undefining CLOCK_REALTIME and CLOCK_MONOTONIC might not address the issue effectively.
#93075
It's probably best to create a function that maps CLOCK_REALTIME and CLOCK_MONOTONIC to SYS_CLOCK_REALTIME and SYS_CLOCK_MONOTONIC.
Some toolchains may define CLOCK_REALTIME and CLOCK_MONOTONIC in their libc headers with values that differ from Zephyr's internal SYS_CLOCK_REALTIME and SYS_CLOCK_MONOTONIC. To ensure consistent behavior across all boards and toolchains, Introduce a helper function to map CLOCK_REALTIME and CLOCK_MONOTONIC to Zephyr's internal clock IDs (SYS_CLOCK_REALTIME and SYS_CLOCK_MONOTONIC). This prevents mismatched clock IDs being passed to the kernel, avoiding invalid clockid errors when using functions like clock_gettime(). Signed-off-by: Mohamed Moawad <moawad@synopsys.com>
M-Moawad
force-pushed
the
moawad-posix-align-clock-ids
branch
from
670b68f to
67928a3
Compare
github-actions
bot
requested review from
andyross,
dcpleung,
nashif and
peter-mitsis
I tested your patch locally with ARC MWDT toolchain and it passed, so I’ve applied it—it’s definitely a cleaner solution. I’ve also triggered a pipeline job to run the full test suite, and I’ll let you know the results soon. |
|
Passed. |
|
@tagunil Please review the current change |
M-Moawad
changed the title
Done |
Some toolchains may define CLOCK_REALTIME and
CLOCK_MONOTONIC in their libc headers
with values that differ from Zephyr's internal
SYS_CLOCK_REALTIME and SYS_CLOCK_MONOTONIC.
To ensure consistent behavior across all toolchains, Introduce a helper function to map
CLOCK_REALTIME and CLOCK_MONOTONIC to Zephyr's internal
clock IDs (SYS_CLOCK_REALTIME and SYS_CLOCK_MONOTONIC).
This prevents mismatched clock IDs being passed to the kernel, avoiding invalid clockid errors when using functions like clock_gettime().
Fixing issue: #93073