Message ID | 20200919130759.31916-2-lukma@denx.de |
---|---|
State | New |
Headers | show |
Series | [v2,1/3] nptl: futex: Move __NR_futex_time64 alias to beginning of futex-internal.h | expand |
On Sat, Sep 19, 2020 at 6:08 AM Lukasz Majewski <lukma@denx.de> 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. Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > > --- > 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 > @@ -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; > + > + 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 (); > + } > +} > 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 */ > 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 > -- > 2.20.1 >
Dear Community, > 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. > Do we have more comments regarding this patch? Or shall I pull it to -master? > --- > 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 > @@ -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; > + > + 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 (); > + } > +} > 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 */ > 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 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
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'. > > --- > 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. > + > + 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?
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
On Sep 30 2020, Lukasz Majewski wrote: > 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). The m68k has 16 registers, which is plenty (x86 has only 8). Andreas.
On 30/09/2020 09:47, Lukasz Majewski wrote: > 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. I recall this was need on first patch for the cancellable 64-bit futex_abstimed_wait where it embedded the 32-bit fallback in the __futex_abstimed_wait_cancelable64. And my suggestion to move it to an external function seems to avoid the extra compiler flag. This patchset uses the same strategy and I checked both patches and it seems that -fno-inline is not required to build m68k.
Hi Andreas, > On Sep 30 2020, Lukasz Majewski wrote: > > > 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). > > The m68k has 16 registers, which is plenty (x86 has only 8). This is what I got from wiki (and no I'm not the expert with m68k, so corrections are welcome): https://en.wikipedia.org/wiki/Motorola_68000_series (8 x 32 bit data registers) And I cannot say why it is not able to generate the correct code... The proposed flag (-fno-inline) seems to be the most pragmatic solution (the code in question is in futex-interna.c and the flag only affects this port). > > Andreas. > 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
Hi Adhemerval, > On 30/09/2020 09:47, Lukasz Majewski wrote: > > 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. > > I recall this was need on first patch for the cancellable 64-bit > futex_abstimed_wait where it embedded the 32-bit fallback in the > __futex_abstimed_wait_cancelable64. And my suggestion to move it to > an external function seems to avoid the extra compiler flag. > > This patchset uses the same strategy and I checked both patches > and it seems that -fno-inline is not required to build m68k. As fair as I can tell (at the time I tested it) - it was necessary to have -fno-inline when this patch was applied on top of the one which is now in the -master branch (in futex-internal.c). I will double check it now with: glibc/glibc-many-build --keep failed glibcs 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
On Wed, 30 Sep 2020 15:39:53 +0200 Lukasz Majewski <lukma@denx.de> wrote: > Hi Adhemerval, > > > On 30/09/2020 09:47, Lukasz Majewski wrote: > > > 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. > > > > I recall this was need on first patch for the cancellable 64-bit > > futex_abstimed_wait where it embedded the 32-bit fallback in the > > __futex_abstimed_wait_cancelable64. And my suggestion to move it to > > an external function seems to avoid the extra compiler flag. > > > > This patchset uses the same strategy and I checked both patches > > and it seems that -fno-inline is not required to build m68k. > > As fair as I can tell (at the time I tested it) - it was necessary to > have -fno-inline when this patch was applied on top of the one which > is now in the -master branch (in futex-internal.c). > > I will double check it now with: > > glibc/glibc-many-build --keep failed glibcs Indeed with current -master the issue is gone: ../src/scripts/build-many-glibcs.py /home/lukma/work/glibc/glibc-many-build --keep failed glibcs m68k-linux-gnu m68k-linux-gnu-coldfire m68k-linux-gnu-coldfire-soft x86_64-linux-gnu Is all OK with the -fno-inline removed.... Strange. > > > 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 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
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 @@ -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; + + 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 (); + } +} 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 */ 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