[PATCH v4] y2038: Convert aio_suspend to support 64 bit time
Lukasz Majewski
lukma@denx.de
Fri Nov 27 14:17:22 GMT 2020
Hi Adhemerval,
> On 27/11/2020 09:56, Lukasz Majewski wrote:
> > The aio_suspend function has been converted to support 64 bit time.
> >
> > This change uses (in aio_misc.h):
> > - __futex_abstimed_wait64 (instead of futex_reltimed_wait)
> > - __futex_abstimed_wait_cancellable64
> > (instead of futex_reltimed_wait_cancellable)
> > from ./sysdeps/nptl/futex-helpers.h
> >
> > The aio_suspend() accepts relative timeout, which then is converted
> > to absolute one.
> >
> > The i686-gnu port (HURD) do not define DONT_NEED_AIO_MISC_COND and
> > as it doesn't (yet) support 64 bit time it uses not converted
> > pthread_cond_timedwait().
> >
> > The __aio_suspend() is supposed to be run on ports with __TIMESIZE
> > !=64 and __WORDSIZE==32. It internally utilizes
> > __aio_suspend_time64() and hence the conversion from 32 bit struct
> > timespec to 64 bit one is required.
> >
> > For ports supporting 64 bit time the __aio_suspend_time64() will be
> > used either via alias (to __aio_suspend when __TIMESIZE==64) or
> > redirection (when -D_TIME_BITS=64 is passed).
> >
> > Build tests:
> > ./src/scripts/build-many-glibcs.py glibcs
> >
> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> > Changes for v2:
> > - Add missing -EOVERFLOW error handling for __futex_reltimed_wait64
> > and _futex_reltimed_wait_cancelable64
> >
> > Changes for v3:
> > - Remove "__" prefix from futex_reltimed_wait64 and
> > futex_reltimed_wait_cancellable64
> >
> > - Remove some code, as HURD is not defining DONT_NEED_AIO_MISC_COND
> > (i.e. Linux ports are defining it) and add in-code explanation why
> > the code is NOT converted to support 64 bit time.
> >
> > - Rewrite the commit message
> >
> > Changes for v4:
> > - Re-base on the newest master after refactor of the futex code -
> > e.g. no need to provide futex_reltimed_wait_cancelable64()
> >
> > - Add missing libpthread_hidden_{proto|def}
>
> LGTM with a small fix below.
>
> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
>
> > ---
> > include/aio.h | 8 +++++
> > nptl/Versions | 1 +
> > sysdeps/nptl/aio_misc.h | 9 +++---
> > sysdeps/nptl/futex-internal.c | 2 ++
> > sysdeps/nptl/futex-internal.h | 6 ++--
> > sysdeps/pthread/aio_suspend.c | 60
> > ++++++++++++++++++++--------------- 6 files changed, 55
> > insertions(+), 31 deletions(-)
> >
> > diff --git a/include/aio.h b/include/aio.h
> > index 90c74f9951..c7f4233310 100644
> > --- a/include/aio.h
> > +++ b/include/aio.h
> > @@ -9,6 +9,14 @@ extern void __aio_init (const struct aioinit
> > *__init); lio_listio and we do not issue events for each individual
> > list element. */
> > #define LIO_NO_INDIVIDUAL_EVENT 128
> > +
> > +# if __TIMESIZE == 64
> > +# define __aio_suspend_time64 __aio_suspend
> > +# else
> > +extern int __aio_suspend_time64 (const struct aiocb *const list[],
> > int nent,
> > + const struct __timespec64
> > *timeout); +librt_hidden_proto (__aio_suspend_time64)
> > +# endif
> > #endif
> >
> > #endif
> > diff --git a/nptl/Versions b/nptl/Versions
> > index aed118e717..02650fe91c 100644
> > --- a/nptl/Versions
> > +++ b/nptl/Versions
> > @@ -302,6 +302,7 @@ libpthread {
> > __pthread_clock_gettime; __pthread_clock_settime;
> > __pthread_unwind; __pthread_get_minstack;
> > __pthread_barrier_init; __pthread_barrier_wait;
> > + __futex_abstimed_wait64; __futex_abstimed_wait_cancelable64;
> > __shm_directory;
> > __libpthread_freeres;
> > }
> > diff --git a/sysdeps/nptl/aio_misc.h b/sysdeps/nptl/aio_misc.h
> > index 3f195f4794..dd8d99e5d6 100644
> > --- a/sysdeps/nptl/aio_misc.h
> > +++ b/sysdeps/nptl/aio_misc.h
> > @@ -45,11 +45,12 @@
> > do
> > \ {
> > \ if (cancel)
> > \
> > - status = futex_reltimed_wait_cancelable (
> > \
> > - (unsigned int *) futexaddr, oldval, timeout,
> > FUTEX_PRIVATE); \
> > + status = __futex_abstimed_wait_cancelable64 (
> > \
> > + (unsigned int *) futexaddr, oldval,
> > CLOCK_MONOTONIC, timeout, \
> > + FUTEX_PRIVATE);
> > \ else
> > \
> > - status = futex_reltimed_wait ((unsigned int *)
> > futexaddr, \
> > - oldval, timeout, FUTEX_PRIVATE);
> > \
> > + status = __futex_abstimed_wait64 ((unsigned int *)
> > futexaddr, \
> > + oldval, CLOCK_REALTIME, timeout, FUTEX_PRIVATE);
> > \ if (status != EAGAIN)
> > \ break;
> > \
>
> It also need to check for EOVERFLOW along with the ETIMEOUT check (to
> return EAGAIN).
But this was already implemented in the patch, which you posted today?
Or shall I do something more?
> \
> > diff --git a/sysdeps/nptl/futex-internal.c
> > b/sysdeps/nptl/futex-internal.c index 11031cc46a..22ef312c2a 100644
> > --- a/sysdeps/nptl/futex-internal.c
> > +++ b/sysdeps/nptl/futex-internal.c
> > @@ -112,6 +112,7 @@ __futex_abstimed_wait64 (unsigned int*
> > futex_word, unsigned int expected, return
> > __futex_abstimed_wait_common64 (futex_word, expected, clockid,
> > abstime, private, false); }
> > +libpthread_hidden_def (__futex_abstimed_wait64)
> >
> > int
> > __futex_abstimed_wait_cancelable64 (unsigned int* futex_word,
> > @@ -122,3 +123,4 @@ __futex_abstimed_wait_cancelable64 (unsigned
> > int* futex_word, return __futex_abstimed_wait_common64 (futex_word,
> > expected, clockid, abstime, private, true);
> > }
> > +libpthread_hidden_def (__futex_abstimed_wait_cancelable64)
> > diff --git a/sysdeps/nptl/futex-internal.h
> > b/sysdeps/nptl/futex-internal.h index e67803952f..1640da0ce8 100644
> > --- a/sysdeps/nptl/futex-internal.h
> > +++ b/sysdeps/nptl/futex-internal.h
> > @@ -405,13 +405,15 @@ int
> > __futex_abstimed_wait_cancelable64 (unsigned int* futex_word,
> > unsigned int expected,
> > clockid_t clockid, const struct __timespec64* abstime,
> > - int private) attribute_hidden;
> > + int private);
> > +libpthread_hidden_proto (__futex_abstimed_wait_cancelable64);
> >
> > int
> > __futex_abstimed_wait64 (unsigned int* futex_word, unsigned int
> > expected, clockid_t clockid,
> > const struct __timespec64* abstime,
> > - int private) attribute_hidden;
> > + int private);
> > +libpthread_hidden_proto (__futex_abstimed_wait64);
> >
> >
> > static __always_inline int
> > diff --git a/sysdeps/pthread/aio_suspend.c
> > b/sysdeps/pthread/aio_suspend.c index ad03f13558..32032a4f44 100644
> > --- a/sysdeps/pthread/aio_suspend.c
> > +++ b/sysdeps/pthread/aio_suspend.c
> > @@ -94,7 +94,7 @@ cleanup (void *arg)
> > #ifdef DONT_NEED_AIO_MISC_COND
> > static int
> > __attribute__ ((noinline))
> > -do_aio_misc_wait (unsigned int *cntr, const struct timespec
> > *timeout) +do_aio_misc_wait (unsigned int *cntr, const struct
> > __timespec64 *timeout) {
> > int result = 0;
> >
> > @@ -105,8 +105,8 @@ do_aio_misc_wait (unsigned int *cntr, const
> > struct timespec *timeout) #endif
> >
> > int
> > -aio_suspend (const struct aiocb *const list[], int nent,
> > - const struct timespec *timeout)
> > +__aio_suspend_time64 (const struct aiocb *const list[], int nent,
> > + const struct __timespec64 *timeout)
> > {
> > if (__glibc_unlikely (nent < 0))
> > {
> > @@ -157,6 +157,18 @@ aio_suspend (const struct aiocb *const list[],
> > int nent, break;
> > }
> >
> > + struct __timespec64 ts;
> > + if (timeout != NULL)
> > + {
> > + __clock_gettime64 (CLOCK_MONOTONIC, &ts);
> > + ts.tv_sec += timeout->tv_sec;
> > + ts.tv_nsec += timeout->tv_nsec;
> > + if (ts.tv_nsec >= 1000000000)
> > + {
> > + ts.tv_nsec -= 1000000000;
> > + ts.tv_sec++;
> > + }
> > + }
> >
> > /* Only if none of the entries is NULL or finished to be wait.
> > */ if (cnt == nent && any)
> > @@ -175,29 +187,11 @@ aio_suspend (const struct aiocb *const
> > list[], int nent, pthread_cleanup_push (cleanup, &clparam);
> >
> > #ifdef DONT_NEED_AIO_MISC_COND
> > - result = do_aio_misc_wait (&cntr, timeout);
> > + result = do_aio_misc_wait (&cntr, timeout == NULL ? NULL :
> > &ts); #else
> > - if (timeout == NULL)
> > - result = pthread_cond_wait (&cond, &__aio_requests_mutex);
> > - else
> > - {
> > - /* We have to convert the relative timeout value into an
> > - absolute time value with pthread_cond_timedwait
> > expects. */
> > - struct timespec now;
> > - struct timespec abstime;
> > -
> > - __clock_gettime (CLOCK_REALTIME, &now);
> > - abstime.tv_nsec = timeout->tv_nsec + now.tv_nsec;
> > - abstime.tv_sec = timeout->tv_sec + now.tv_sec;
> > - if (abstime.tv_nsec >= 1000000000)
> > - {
> > - abstime.tv_nsec -= 1000000000;
> > - abstime.tv_sec += 1;
> > - }
> > -
> > - result = pthread_cond_timedwait (&cond,
> > &__aio_requests_mutex,
> > - &abstime);
> > - }
> > + struct timespec ts32 = valid_timespec64_to_timespec (ts);
> > + result = pthread_cond_timedwait (&cond,
> > &__aio_requests_mutex,
> > + timeout == NULL ? NULL :
> > &ts32); #endif
> >
> > pthread_cleanup_pop (0);
> > @@ -250,4 +244,20 @@ aio_suspend (const struct aiocb *const list[],
> > int nent, return result;
> > }
> >
> > +#if __TIMESIZE != 64
> > +librt_hidden_def (__aio_suspend_time64)
> > +
> > +int
> > +__aio_suspend (const struct aiocb *const list[], int nent,
> > + const struct timespec *timeout)
> > +{
> > + struct __timespec64 ts64;
> > +
> > + if (timeout != NULL)
> > + ts64 = valid_timespec_to_timespec64 (*timeout);
> > +
> > + return __aio_suspend_time64 (list, nent, timeout != NULL ? &ts64
> > : NULL); +}
> > +#endif
> > +weak_alias (__aio_suspend, aio_suspend)
> > weak_alias (aio_suspend, aio_suspend64)
> >
Best regards,
Lukasz Majewski
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <https://sourceware.org/pipermail/libc-alpha/attachments/20201127/7bb7aa33/attachment-0001.sig>
More information about the Libc-alpha
mailing list