diff mbox series

aio_suspend64: Fix clock discrepancy

Message ID 20250309221155.937522-1-samuel.thibault@ens-lyon.org (mailing list archive)
State Superseded
Headers
Series aio_suspend64: Fix clock discrepancy |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
redhat-pt-bot/TryBot-32bit fail Patch caused testsuite regressions
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm fail Test failed
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 fail Test failed

Commit Message

Samuel Thibault March 9, 2025, 10:11 p.m. UTC 
  cc5d5852c65e ("y2038: Convert aio_suspend to support 64 bit time")
switched from __clock_gettime (CLOCK_REALTIME, &now); to __clock_gettime64
(CLOCK_MONOTONIC, &ts);, but pthread_cond_timedwait is based on the
absolute realtime clock, not the monotonic clock.
---
 rt/aio_suspend.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Florian Weimer March 9, 2025, 10:32 p.m. UTC | #1 
* Samuel Thibault:

> cc5d5852c65e ("y2038: Convert aio_suspend to support 64 bit time")
> switched from __clock_gettime (CLOCK_REALTIME, &now); to __clock_gettime64
> (CLOCK_MONOTONIC, &ts);, but pthread_cond_timedwait is based on the
> absolute realtime clock, not the monotonic clock.
> ---
>  rt/aio_suspend.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/rt/aio_suspend.c b/rt/aio_suspend.c
> index 2257cd80d4..49a86f6ab3 100644
> --- a/rt/aio_suspend.c
> +++ b/rt/aio_suspend.c
> @@ -161,7 +161,7 @@ ___aio_suspend_time64 (const struct aiocb *const list[], int nent,
>    struct __timespec64 ts;
>    if (timeout != NULL)
>      {
> -      __clock_gettime64 (CLOCK_MONOTONIC, &ts);
> +      __clock_gettime64 (CLOCK_REALTIME, &ts);
>        ts.tv_sec += timeout->tv_sec;
>        ts.tv_nsec += timeout->tv_nsec;
>        if (ts.tv_nsec >= 1000000000)

Shouldn't this be fixed in the other direction, using
pthread_cond_clockwait with CLOCK_MONOTONIC?

Either way, I think this needs a bug in Bugzilla.
Samuel Thibault March 14, 2025, 7:55 p.m. UTC | #2 
Florian Weimer, le dim. 09 mars 2025 23:32:28 +0100, a ecrit:
> > index 2257cd80d4..49a86f6ab3 100644
> > --- a/rt/aio_suspend.c
> > +++ b/rt/aio_suspend.c
> > @@ -161,7 +161,7 @@ ___aio_suspend_time64 (const struct aiocb *const list[], int nent,
> >    struct __timespec64 ts;
> >    if (timeout != NULL)
> >      {
> > -      __clock_gettime64 (CLOCK_MONOTONIC, &ts);
> > +      __clock_gettime64 (CLOCK_REALTIME, &ts);
> >        ts.tv_sec += timeout->tv_sec;
> >        ts.tv_nsec += timeout->tv_nsec;
> >        if (ts.tv_nsec >= 1000000000)
> 
> Shouldn't this be fixed in the other direction, using
> pthread_cond_clockwait with CLOCK_MONOTONIC?

Ah, I hadn't found it in the manpage, fixed so.

> Either way, I think this needs a bug in Bugzilla.

Ah, I missed filing it before sending the newer patch on the list, I
have fixed my local commit to close
https://sourceware.org/bugzilla/show_bug.cgi?id=32795

Samuel
diff mbox series

Patch

diff --git a/rt/aio_suspend.c b/rt/aio_suspend.c
index 2257cd80d4..49a86f6ab3 100644
--- a/rt/aio_suspend.c
+++ b/rt/aio_suspend.c
@@ -161,7 +161,7 @@  ___aio_suspend_time64 (const struct aiocb *const list[], int nent,
   struct __timespec64 ts;
   if (timeout != NULL)
     {
-      __clock_gettime64 (CLOCK_MONOTONIC, &ts);
+      __clock_gettime64 (CLOCK_REALTIME, &ts);
       ts.tv_sec += timeout->tv_sec;
       ts.tv_nsec += timeout->tv_nsec;
       if (ts.tv_nsec >= 1000000000)