[PATCH v2 2/3] Y2038: nptl: Provide futex_abstimed_wait64 supporting 64 bit time
Lukasz Majewski
lukma@denx.de
Wed Sep 30 12:47:10 GMT 2020
Hi Adhemerval,
> On 19/09/2020 10:07, Lukasz Majewski wrote:
> > This is the helper function, which uses struct __timespec64
> > to provide 64 bit absolute time to futex syscalls.
> >
> > The aim of this function is to move convoluted pre-processor
> > macro code from sysdeps/nptl/lowlevellock-futex.h to C
> > function in futex-internal.c
> >
> > The futex_abstimed_wait64 function has been put into a separate
> > file on the purpose - to avoid issues apparent on the m68k
> > architecture related to small number of available registers (there
> > is not enough registers to put all necessary arguments in them if
> > the above function would be added to futex-internal.h with
> > __always_inline attribute).
> >
> > Additional precautions for m68k port have been taken - the
> > futex-internal.c file will be compiled with -fno-inline flag.
>
> LGTM with a nit regarding style and a clarification about
> '-fno-inline'.
Please find the explanation below.
>
> >
> > ---
> > Changes for v2:
> > - Handle the case when *abstime pointer is NULL
> > ---
> > sysdeps/nptl/futex-internal.c | 70
> > +++++++++++++++++++++++++++ sysdeps/nptl/futex-internal.h |
> > 6 +++ sysdeps/unix/sysv/linux/m68k/Makefile | 2 +
> > 3 files changed, 78 insertions(+)
> >
> > diff --git a/sysdeps/nptl/futex-internal.c
> > b/sysdeps/nptl/futex-internal.c index 3366aac162..3211b4c94f 100644
> > --- a/sysdeps/nptl/futex-internal.c
> > +++ b/sysdeps/nptl/futex-internal.c
> > @@ -45,6 +45,29 @@ __futex_abstimed_wait_cancelable32 (unsigned
> > int* futex_word, abstime != NULL ? &ts32 : NULL,
> > NULL /* Unused. */,
> > FUTEX_BITSET_MATCH_ANY); }
> > +
> > +static int
> > +__futex_abstimed_wait32 (unsigned int* futex_word,
> > + unsigned int expected, clockid_t clockid,
> > + const struct __timespec64* abstime,
> > + int private)
> > +{
> > + struct timespec ts32;
> > +
> > + if (abstime != NULL && ! in_time_t_range (abstime->tv_sec))
> > + return -EOVERFLOW;
> > +
> > + unsigned int clockbit = (clockid == CLOCK_REALTIME) ?
> > + FUTEX_CLOCK_REALTIME : 0;
> > + int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit,
> > private); +
> > + if (abstime != NULL)
> > + ts32 = valid_timespec64_to_timespec (*abstime);
> > +
> > + return INTERNAL_SYSCALL_CALL (futex, futex_word, op, expected,
> > + abstime != NULL ? &ts32 : NULL,
> > + NULL /* Unused. */,
> > FUTEX_BITSET_MATCH_ANY); +}
> > #endif
> >
> > int
>
> Ok.
>
> > @@ -97,3 +120,50 @@ __futex_abstimed_wait_cancelable64 (unsigned
> > int* futex_word, futex_fatal_error ();
> > }
> > }
> > +
> > +int
> > +__futex_abstimed_wait64 (unsigned int* futex_word, unsigned int
> > expected,
> > + clockid_t clockid,
> > + const struct __timespec64* abstime, int
> > private) +{
> > + unsigned int clockbit;
> > + int err;
> > +
> > + /* Work around the fact that the kernel rejects negative timeout
> > values
> > + despite them being valid. */
> > + if (__glibc_unlikely ((abstime != NULL) && (abstime->tv_sec <
> > 0)))
> > + return ETIMEDOUT;
> > +
> > + if (! lll_futex_supported_clockid(clockid))
> > + return EINVAL;
>
> Space before parentheses.
Ok. Fixed.
>
> > +
> > + clockbit = (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME :
> > 0;
> > + int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit,
> > private); +
> > + err = INTERNAL_SYSCALL_CALL (futex_time64, futex_word, op,
> > expected,
> > + abstime, NULL /* Unused. */,
> > + FUTEX_BITSET_MATCH_ANY);
> > +#ifndef __ASSUME_TIME64_SYSCALLS
> > + if (err == -ENOSYS)
> > + err = __futex_abstimed_wait32 (futex_word, expected,
> > + clockid, abstime, private);
> > +#endif
> > + switch (err)
> > + {
> > + case 0:
> > + case -EAGAIN:
> > + case -EINTR:
> > + case -ETIMEDOUT:
> > + return -err;
> > +
> > + case -EFAULT: /* Must have been caused by a glibc or
> > application bug. */
> > + case -EINVAL: /* Either due to wrong alignment, unsupported
> > + clockid or due to the timeout not being
> > + normalized. Must have been caused by a glibc
> > or
> > + application bug. */
> > + case -ENOSYS: /* Must have been caused by a glibc bug. */
> > + /* No other errors are documented at this time. */
> > + default:
> > + futex_fatal_error ();
> > + }
> > +}
>
> Ok.
>
> > diff --git a/sysdeps/nptl/futex-internal.h
> > b/sysdeps/nptl/futex-internal.h index 7f3910ad98..1ba0d61938 100644
> > --- a/sysdeps/nptl/futex-internal.h
> > +++ b/sysdeps/nptl/futex-internal.h
> > @@ -529,4 +529,10 @@ __futex_abstimed_wait_cancelable64 (unsigned
> > int* futex_word, const struct __timespec64* abstime,
> > int private) attribute_hidden;
> >
> > +int
> > +__futex_abstimed_wait64 (unsigned int* futex_word, unsigned int
> > expected,
> > + clockid_t clockid,
> > + const struct __timespec64* abstime,
> > + int private) attribute_hidden;
> > +
> > #endif /* futex-internal.h */
>
> Ok.
>
> > diff --git a/sysdeps/unix/sysv/linux/m68k/Makefile
> > b/sysdeps/unix/sysv/linux/m68k/Makefile index
> > be40fae68a..65164c5752 100644 ---
> > a/sysdeps/unix/sysv/linux/m68k/Makefile +++
> > b/sysdeps/unix/sysv/linux/m68k/Makefile @@ -21,3 +21,5 @@
> > sysdep-dl-routines += dl-static sysdep-others += lddlibc4
> > install-bin += lddlibc4
> > endif
> > +
> > +CFLAGS-futex-internal.c += -fno-inline
> >
>
> Do we still need it?
Unfortunately, yes. The m68k architecture has the issue with properly
compiling this code (in futex-internal.c) as it has very limited number
of registers (8 x 32 bit IIRC).
I did some investigation and it looks like static inline
in_time_t_range() function affects the compiler capabilities to
generate correct code.
It can be easily inlined on any architecture but m68k.
As m68k has issues with this code compilation - it is IMHO better to
disable inlining (-fno-inline) on this particular port.
As a result we would have slower execution only on relevant functions,
but 64 bit time support would be provided.
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/20200930/424c71ce/attachment.sig>
More information about the Libc-alpha
mailing list