* [PATCH] y2038: nptl: Provide __futex_clock_wait_bitset64 to support 64 bit bitset
@ 2020-10-19 14:57 Lukasz Majewski
2020-10-20 11:01 ` Stefan Liebler via Libc-alpha
0 siblings, 1 reply; 4+ messages in thread
From: Lukasz Majewski @ 2020-10-19 14:57 UTC (permalink / raw)
To: Joseph Myers, Paul Eggert, Adhemerval Zanella
Cc: Florian Weimer, Stefan Liebler, GNU C Library, Andreas Schwab,
Stepan Golosunov, Alistair Francis
The commit:
"y2038: nptl: Convert pthread_mutex_{clock|timed}lock to support 64 bit"
SHA1: 29e9874a048f47e2d46c40253036c8d2de921548
introduced support for 64 bit timeouts. Unfortunately, it was missing the
code for bitset - i.e. lll_futex_clock_wait_bitset C preprocessor macro
was used. As a result the 64 bit struct __timespec64 was coerced to 32
bit struct timespec and regression visible as timeout was observed
(nptl/tst-robust10 on s390).
Reported-by: Stefan Liebler <stli@linux.ibm.com>
---
nptl/pthread_mutex_timedlock.c | 2 +-
sysdeps/nptl/futex-internal.c | 48 +++++++++++++++++++++++++++++++++-
sysdeps/nptl/futex-internal.h | 5 ++++
3 files changed, 53 insertions(+), 2 deletions(-)
diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c
index dc40399f02..fe9e651f6c 100644
--- a/nptl/pthread_mutex_timedlock.c
+++ b/nptl/pthread_mutex_timedlock.c
@@ -268,7 +268,7 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
assume_other_futex_waiters |= FUTEX_WAITERS;
/* Block using the futex. */
- int err = lll_futex_clock_wait_bitset (&mutex->__data.__lock,
+ int err = __futex_clock_wait_bitset64 (&mutex->__data.__lock,
oldval, clockid, abstime,
PTHREAD_ROBUST_MUTEX_PSHARED (mutex));
/* The futex call timed out. */
diff --git a/sysdeps/nptl/futex-internal.c b/sysdeps/nptl/futex-internal.c
index 2e1919f056..a978ad0ad2 100644
--- a/sysdeps/nptl/futex-internal.c
+++ b/sysdeps/nptl/futex-internal.c
@@ -70,7 +70,28 @@ __futex_abstimed_wait32 (unsigned int* futex_word,
abstime != NULL ? &ts32 : NULL,
NULL /* Unused. */, FUTEX_BITSET_MATCH_ANY);
}
-#endif
+
+static int
+__futex_clock_wait_bitset32 (int *futexp, int val, clockid_t clockid,
+ const struct __timespec64 *abstime, int private)
+{
+ struct timespec ts32;
+
+ if (abstime != NULL && ! in_time_t_range (abstime->tv_sec))
+ return -EOVERFLOW;
+
+ const unsigned int clockbit =
+ (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME : 0;
+ const int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit, private);
+
+ if (abstime != NULL)
+ ts32 = valid_timespec64_to_timespec (*abstime);
+
+ return INTERNAL_SYSCALL_CALL (futex, futexp, op, val,
+ abstime != NULL ? &ts32 : NULL,
+ NULL /* Unused. */, FUTEX_BITSET_MATCH_ANY);
+}
+#endif /* ! __ASSUME_TIME64_SYSCALLS */
int
__futex_abstimed_wait_cancelable64 (unsigned int* futex_word,
@@ -222,3 +243,28 @@ __futex_clocklock_wait64 (int *futex, int val, clockid_t clockid,
return -err;
}
+
+int
+__futex_clock_wait_bitset64 (int *futexp, int val, clockid_t clockid,
+ const struct __timespec64 *abstime,
+ int private)
+{
+ long int ret;
+ if (! lll_futex_supported_clockid (clockid))
+ {
+ return -EINVAL;
+ }
+
+ const unsigned int clockbit =
+ (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME : 0;
+ const int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit, private);
+
+ ret = INTERNAL_SYSCALL_CALL (futex_time64, futexp, op, val,
+ abstime, NULL /* Unused. */,
+ FUTEX_BITSET_MATCH_ANY);
+#ifndef __ASSUME_TIME64_SYSCALLS
+ if (ret == -ENOSYS)
+ ret = __futex_clock_wait_bitset32 (futexp, val, clockid, abstime, private);
+#endif
+ return ret;
+}
diff --git a/sysdeps/nptl/futex-internal.h b/sysdeps/nptl/futex-internal.h
index 8a5f62768f..cd356e4fa8 100644
--- a/sysdeps/nptl/futex-internal.h
+++ b/sysdeps/nptl/futex-internal.h
@@ -603,4 +603,9 @@ __futex_clocklock64 (int *futex, clockid_t clockid,
return err;
}
+int
+__futex_clock_wait_bitset64 (int *futexp, int val, clockid_t clockid,
+ const struct __timespec64 *abstime,
+ int private) attribute_hidden;
+
#endif /* futex-internal.h */
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] y2038: nptl: Provide __futex_clock_wait_bitset64 to support 64 bit bitset
2020-10-19 14:57 [PATCH] y2038: nptl: Provide __futex_clock_wait_bitset64 to support 64 bit bitset Lukasz Majewski
@ 2020-10-20 11:01 ` Stefan Liebler via Libc-alpha
2020-10-20 13:31 ` Lukasz Majewski
0 siblings, 1 reply; 4+ messages in thread
From: Stefan Liebler via Libc-alpha @ 2020-10-20 11:01 UTC (permalink / raw)
To: Lukasz Majewski, Joseph Myers, Paul Eggert, Adhemerval Zanella
Cc: Florian Weimer, GNU C Library, Andreas Schwab, Stepan Golosunov,
Alistair Francis
On 10/19/20 4:57 PM, Lukasz Majewski wrote:
> The commit:
> "y2038: nptl: Convert pthread_mutex_{clock|timed}lock to support 64 bit"
> SHA1: 29e9874a048f47e2d46c40253036c8d2de921548
>
> introduced support for 64 bit timeouts. Unfortunately, it was missing the
> code for bitset - i.e. lll_futex_clock_wait_bitset C preprocessor macro
> was used. As a result the 64 bit struct __timespec64 was coerced to 32
> bit struct timespec and regression visible as timeout was observed
> (nptl/tst-robust10 on s390).
>
> Reported-by: Stefan Liebler <stli@linux.ibm.com>
I've successfully run some tests on s390 (31bit) / x86 (32bit) with this
patch.
- strace output of nptl/tst-robust10 on s390 (31bit):
10:41:37.553933 futex_time64(0x406144,
FUTEX_WAIT_BITSET|FUTEX_CLOCK_REALTIME, 2150823950, {tv_sec=1603183298,
tv_nsec=553918404}, FUTEX_BITSET_MATCH_ANY) = -1 ETIMEDOUT (Connection
timed out) <1.000090>
=> Now there is only this single FUTEX_WAIT_BITSET call which times out
after a second.
- strace output of nptl/tst-robust10 on x86 (32bit):
10:45:50.985125 futex_time64(0x804f0f4,
FUTEX_WAIT_BITSET|FUTEX_CLOCK_REALTIME, 2147912575, {tv_sec=1603183551,
tv_nsec=985109825}, FUTEX_BITSET_MATCH_ANY) = -1 ETIMEDOUT (Connection
timed out) <1.000439>
=> Now tv_nsec is not zero anymore and it times out after a second.
I have one question - see below.
Thanks,
Stefan
> ---
> nptl/pthread_mutex_timedlock.c | 2 +-
> sysdeps/nptl/futex-internal.c | 48 +++++++++++++++++++++++++++++++++-
> sysdeps/nptl/futex-internal.h | 5 ++++
> 3 files changed, 53 insertions(+), 2 deletions(-)
>
> diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c
> index dc40399f02..fe9e651f6c 100644
> --- a/nptl/pthread_mutex_timedlock.c
> +++ b/nptl/pthread_mutex_timedlock.c
> @@ -268,7 +268,7 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
> assume_other_futex_waiters |= FUTEX_WAITERS;
>
> /* Block using the futex. */
> - int err = lll_futex_clock_wait_bitset (&mutex->__data.__lock,
> + int err = __futex_clock_wait_bitset64 (&mutex->__data.__lock,
> oldval, clockid, abstime,
> PTHREAD_ROBUST_MUTEX_PSHARED (mutex));
> /* The futex call timed out. */
> diff --git a/sysdeps/nptl/futex-internal.c b/sysdeps/nptl/futex-internal.c
> index 2e1919f056..a978ad0ad2 100644
> --- a/sysdeps/nptl/futex-internal.c
> +++ b/sysdeps/nptl/futex-internal.c
> @@ -70,7 +70,28 @@ __futex_abstimed_wait32 (unsigned int* futex_word,
> abstime != NULL ? &ts32 : NULL,
> NULL /* Unused. */, FUTEX_BITSET_MATCH_ANY);
> }
> -#endif
> +
> +static int
> +__futex_clock_wait_bitset32 (int *futexp, int val, clockid_t clockid,
> + const struct __timespec64 *abstime, int private)
> +{
> + struct timespec ts32;
> +
> + if (abstime != NULL && ! in_time_t_range (abstime->tv_sec))
> + return -EOVERFLOW;
> +
> + const unsigned int clockbit =
> + (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME : 0;
> + const int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit, private);
> +
> + if (abstime != NULL)
> + ts32 = valid_timespec64_to_timespec (*abstime);
> +
> + return INTERNAL_SYSCALL_CALL (futex, futexp, op, val,
> + abstime != NULL ? &ts32 : NULL,
> + NULL /* Unused. */, FUTEX_BITSET_MATCH_ANY);
> +}
> +#endif /* ! __ASSUME_TIME64_SYSCALLS */
>
> int
> __futex_abstimed_wait_cancelable64 (unsigned int* futex_word,
> @@ -222,3 +243,28 @@ __futex_clocklock_wait64 (int *futex, int val, clockid_t clockid,
>
> return -err;
> }
> +
> +int
> +__futex_clock_wait_bitset64 (int *futexp, int val, clockid_t clockid,
> + const struct __timespec64 *abstime,
> + int private)
> +{
> + long int ret;
Is there a reason why long int is used instead of int (which is also
returned by this function)?
> + if (! lll_futex_supported_clockid (clockid))
> + {
> + return -EINVAL;
> + }
> +
> + const unsigned int clockbit =
> + (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME : 0;
> + const int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit, private);
> +
> + ret = INTERNAL_SYSCALL_CALL (futex_time64, futexp, op, val,
> + abstime, NULL /* Unused. */,
> + FUTEX_BITSET_MATCH_ANY);
> +#ifndef __ASSUME_TIME64_SYSCALLS
> + if (ret == -ENOSYS)
> + ret = __futex_clock_wait_bitset32 (futexp, val, clockid, abstime, private);
> +#endif
> + return ret;
> +}
> diff --git a/sysdeps/nptl/futex-internal.h b/sysdeps/nptl/futex-internal.h
> index 8a5f62768f..cd356e4fa8 100644
> --- a/sysdeps/nptl/futex-internal.h
> +++ b/sysdeps/nptl/futex-internal.h
> @@ -603,4 +603,9 @@ __futex_clocklock64 (int *futex, clockid_t clockid,
> return err;
> }
>
> +int
> +__futex_clock_wait_bitset64 (int *futexp, int val, clockid_t clockid,
> + const struct __timespec64 *abstime,
> + int private) attribute_hidden;
> +
> #endif /* futex-internal.h */
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] y2038: nptl: Provide __futex_clock_wait_bitset64 to support 64 bit bitset
2020-10-20 11:01 ` Stefan Liebler via Libc-alpha
@ 2020-10-20 13:31 ` Lukasz Majewski
2020-10-21 6:49 ` Stefan Liebler via Libc-alpha
0 siblings, 1 reply; 4+ messages in thread
From: Lukasz Majewski @ 2020-10-20 13:31 UTC (permalink / raw)
To: Stefan Liebler
Cc: Florian Weimer, GNU C Library, Andreas Schwab, Stepan Golosunov,
Alistair Francis, Joseph Myers
[-- Attachment #1: Type: text/plain, Size: 6885 bytes --]
Hi Stefan,
Thank you for testing.
> On 10/19/20 4:57 PM, Lukasz Majewski wrote:
> > The commit:
> > "y2038: nptl: Convert pthread_mutex_{clock|timed}lock to support 64
> > bit" SHA1: 29e9874a048f47e2d46c40253036c8d2de921548
> >
> > introduced support for 64 bit timeouts. Unfortunately, it was
> > missing the code for bitset - i.e. lll_futex_clock_wait_bitset C
> > preprocessor macro was used. As a result the 64 bit struct
> > __timespec64 was coerced to 32 bit struct timespec and regression
> > visible as timeout was observed (nptl/tst-robust10 on s390).
> >
> > Reported-by: Stefan Liebler <stli@linux.ibm.com>
>
> I've successfully run some tests on s390 (31bit) / x86 (32bit) with
> this patch.
> - strace output of nptl/tst-robust10 on s390 (31bit):
> 10:41:37.553933 futex_time64(0x406144,
> FUTEX_WAIT_BITSET|FUTEX_CLOCK_REALTIME, 2150823950,
> {tv_sec=1603183298, tv_nsec=553918404}, FUTEX_BITSET_MATCH_ANY) = -1
> ETIMEDOUT (Connection timed out) <1.000090>
> => Now there is only this single FUTEX_WAIT_BITSET call which times
> out after a second.
The tv_sec=1603183298 now seems to be correct.
>
> - strace output of nptl/tst-robust10 on x86 (32bit):
> 10:45:50.985125 futex_time64(0x804f0f4,
> FUTEX_WAIT_BITSET|FUTEX_CLOCK_REALTIME, 2147912575,
> {tv_sec=1603183551, tv_nsec=985109825}, FUTEX_BITSET_MATCH_ANY) = -1
> ETIMEDOUT (Connection timed out) <1.000439>
> => Now tv_nsec is not zero anymore and it times out after a second.
>
> I have one question - see below.
>
> Thanks,
> Stefan
>
> > ---
> > nptl/pthread_mutex_timedlock.c | 2 +-
> > sysdeps/nptl/futex-internal.c | 48
> > +++++++++++++++++++++++++++++++++- sysdeps/nptl/futex-internal.h |
> > 5 ++++ 3 files changed, 53 insertions(+), 2 deletions(-)
> >
> > diff --git a/nptl/pthread_mutex_timedlock.c
> > b/nptl/pthread_mutex_timedlock.c index dc40399f02..fe9e651f6c 100644
> > --- a/nptl/pthread_mutex_timedlock.c
> > +++ b/nptl/pthread_mutex_timedlock.c
> > @@ -268,7 +268,7 @@ __pthread_mutex_clocklock_common
> > (pthread_mutex_t *mutex, assume_other_futex_waiters |=
> > FUTEX_WAITERS;
> > /* Block using the futex. */
> > - int err = lll_futex_clock_wait_bitset
> > (&mutex->__data.__lock,
> > + int err = __futex_clock_wait_bitset64
> > (&mutex->__data.__lock, oldval, clockid, abstime,
> > PTHREAD_ROBUST_MUTEX_PSHARED (mutex));
> > /* The futex call timed out. */
> > diff --git a/sysdeps/nptl/futex-internal.c
> > b/sysdeps/nptl/futex-internal.c index 2e1919f056..a978ad0ad2 100644
> > --- a/sysdeps/nptl/futex-internal.c
> > +++ b/sysdeps/nptl/futex-internal.c
> > @@ -70,7 +70,28 @@ __futex_abstimed_wait32 (unsigned int*
> > futex_word, abstime != NULL ? &ts32 : NULL,
> > NULL /* Unused. */,
> > FUTEX_BITSET_MATCH_ANY); }
> > -#endif
> > +
> > +static int
> > +__futex_clock_wait_bitset32 (int *futexp, int val, clockid_t
> > clockid,
> > + const struct __timespec64 *abstime,
> > int private) +{
> > + struct timespec ts32;
> > +
> > + if (abstime != NULL && ! in_time_t_range (abstime->tv_sec))
> > + return -EOVERFLOW;
> > +
> > + const unsigned int clockbit =
> > + (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME : 0;
> > + const int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit,
> > private); +
> > + if (abstime != NULL)
> > + ts32 = valid_timespec64_to_timespec (*abstime);
> > +
> > + return INTERNAL_SYSCALL_CALL (futex, futexp, op, val,
> > + abstime != NULL ? &ts32 : NULL,
> > + NULL /* Unused. */,
> > FUTEX_BITSET_MATCH_ANY); +}
> > +#endif /* ! __ASSUME_TIME64_SYSCALLS */
> >
> > int
> > __futex_abstimed_wait_cancelable64 (unsigned int* futex_word,
> > @@ -222,3 +243,28 @@ __futex_clocklock_wait64 (int *futex, int val,
> > clockid_t clockid,
> > return -err;
> > }
> > +
> > +int
> > +__futex_clock_wait_bitset64 (int *futexp, int val, clockid_t
> > clockid,
> > + const struct __timespec64 *abstime,
> > + int private)
> > +{
> > + long int relowlevellock-futex.ht;
> Is there a reason why long int is used instead of int (which is also
> returned by this function)?
The "long int" type for ret was in the original
lll_futex_clock_wait_bitset() macro in:
./sysdeps/nptl/lowlevellock-futex.h (line 102).
It is also returned when this macro is pasted.
The reason may be that lll_syscall_futex() macro also uses long int to
get return value from INTERNAL_SYSCALL() (Line 70 in the above file).
Then depending on architecture either long or int are used to get the
status of the syscall.
However, in the end the value returned by lll_futex_clock_wait_bitset()
is casted to int anyway:
git grep -n "= lll_futex_clock_wait_bitset"
nptl/pthread_mutex_timedlock.c:271:
int err = lll_futex_clock_wait_bitset (&mutex->__data.__lock,
sysdeps/nptl/futex-internal.h:288:
int err = lll_futex_clock_wait_bitset (futex_word, expected,
sysdeps/nptl/futex-internal.h:324:
int err = lll_futex_clock_wait_bitset (futex_word, expected,
Hence, IMHO it would be correct to change 'long int' to 'int'. Or am I
wrong?
> > + if (! lll_futex_supported_clockid (clockid))
> > + {
> > + return -EINVAL;
> > + }
> > +
> > + const unsigned int clockbit =
> > + (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME : 0;
> > + const int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit,
> > private); +
> > + ret = INTERNAL_SYSCALL_CALL (futex_time64, futexp, op, val,
> > + abstime, NULL /* Unused. */,
> > + FUTEX_BITSET_MATCH_ANY);
> > +#ifndef __ASSUME_TIME64_SYSCALLS
> > + if (ret == -ENOSYS)
> > + ret = __futex_clock_wait_bitset32 (futexp, val, clockid,
> > abstime, private); +#endif
> > + return ret;
> > +}
> > diff --git a/sysdeps/nptl/futex-internal.h
> > b/sysdeps/nptl/futex-internal.h index 8a5f62768f..cd356e4fa8 100644
> > --- a/sysdeps/nptl/futex-internal.h
> > +++ b/sysdeps/nptl/futex-internal.h
> > @@ -603,4 +603,9 @@ __futex_clocklock64 (int *futex, clockid_t
> > clockid, return err;
> > }
> >
> > +int
> > +__futex_clock_wait_bitset64 (int *futexp, int val, clockid_t
> > clockid,
> > + const struct __timespec64 *abstime,
> > + int private) attribute_hidden;
> > +
> > #endif /* futex-internal.h */
> >
>
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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] y2038: nptl: Provide __futex_clock_wait_bitset64 to support 64 bit bitset
2020-10-20 13:31 ` Lukasz Majewski
@ 2020-10-21 6:49 ` Stefan Liebler via Libc-alpha
0 siblings, 0 replies; 4+ messages in thread
From: Stefan Liebler via Libc-alpha @ 2020-10-21 6:49 UTC (permalink / raw)
To: Lukasz Majewski
Cc: Florian Weimer, GNU C Library, Andreas Schwab, Stepan Golosunov,
Alistair Francis, Joseph Myers
On 10/20/20 3:31 PM, Lukasz Majewski wrote:
> Hi Stefan,
>
> Thank you for testing.
>
>> On 10/19/20 4:57 PM, Lukasz Majewski wrote:
>>> The commit:
>>> "y2038: nptl: Convert pthread_mutex_{clock|timed}lock to support 64
>>> bit" SHA1: 29e9874a048f47e2d46c40253036c8d2de921548
>>>
>>> introduced support for 64 bit timeouts. Unfortunately, it was
>>> missing the code for bitset - i.e. lll_futex_clock_wait_bitset C
>>> preprocessor macro was used. As a result the 64 bit struct
>>> __timespec64 was coerced to 32 bit struct timespec and regression
>>> visible as timeout was observed (nptl/tst-robust10 on s390).
>>>
>>> Reported-by: Stefan Liebler <stli@linux.ibm.com>
>>
>> I've successfully run some tests on s390 (31bit) / x86 (32bit) with
>> this patch.
>> - strace output of nptl/tst-robust10 on s390 (31bit):
>> 10:41:37.553933 futex_time64(0x406144,
>> FUTEX_WAIT_BITSET|FUTEX_CLOCK_REALTIME, 2150823950,
>> {tv_sec=1603183298, tv_nsec=553918404}, FUTEX_BITSET_MATCH_ANY) = -1
>> ETIMEDOUT (Connection timed out) <1.000090>
>> => Now there is only this single FUTEX_WAIT_BITSET call which times
>> out after a second.
>
> The tv_sec=1603183298 now seems to be correct.
>
>>
>> - strace output of nptl/tst-robust10 on x86 (32bit):
>> 10:45:50.985125 futex_time64(0x804f0f4,
>> FUTEX_WAIT_BITSET|FUTEX_CLOCK_REALTIME, 2147912575,
>> {tv_sec=1603183551, tv_nsec=985109825}, FUTEX_BITSET_MATCH_ANY) = -1
>> ETIMEDOUT (Connection timed out) <1.000439>
>> => Now tv_nsec is not zero anymore and it times out after a second.
>>
>> I have one question - see below.
>>
>> Thanks,
>> Stefan
>>
>>> ---
>>> nptl/pthread_mutex_timedlock.c | 2 +-
>>> sysdeps/nptl/futex-internal.c | 48
>>> +++++++++++++++++++++++++++++++++- sysdeps/nptl/futex-internal.h |
>>> 5 ++++ 3 files changed, 53 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/nptl/pthread_mutex_timedlock.c
>>> b/nptl/pthread_mutex_timedlock.c index dc40399f02..fe9e651f6c 100644
>>> --- a/nptl/pthread_mutex_timedlock.c
>>> +++ b/nptl/pthread_mutex_timedlock.c
>>> @@ -268,7 +268,7 @@ __pthread_mutex_clocklock_common
>>> (pthread_mutex_t *mutex, assume_other_futex_waiters |=
>>> FUTEX_WAITERS;
>>> /* Block using the futex. */
>>> - int err = lll_futex_clock_wait_bitset
>>> (&mutex->__data.__lock,
>>> + int err = __futex_clock_wait_bitset64
>>> (&mutex->__data.__lock, oldval, clockid, abstime,
>>> PTHREAD_ROBUST_MUTEX_PSHARED (mutex));
>>> /* The futex call timed out. */
>>> diff --git a/sysdeps/nptl/futex-internal.c
>>> b/sysdeps/nptl/futex-internal.c index 2e1919f056..a978ad0ad2 100644
>>> --- a/sysdeps/nptl/futex-internal.c
>>> +++ b/sysdeps/nptl/futex-internal.c
>>> @@ -70,7 +70,28 @@ __futex_abstimed_wait32 (unsigned int*
>>> futex_word, abstime != NULL ? &ts32 : NULL,
>>> NULL /* Unused. */,
>>> FUTEX_BITSET_MATCH_ANY); }
>>> -#endif
>>> +
>>> +static int
>>> +__futex_clock_wait_bitset32 (int *futexp, int val, clockid_t
>>> clockid,
>>> + const struct __timespec64 *abstime,
>>> int private) +{
>>> + struct timespec ts32;
>>> +
>>> + if (abstime != NULL && ! in_time_t_range (abstime->tv_sec))
>>> + return -EOVERFLOW;
>>> +
>>> + const unsigned int clockbit =
>>> + (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME : 0;
>>> + const int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit,
>>> private); +
>>> + if (abstime != NULL)
>>> + ts32 = valid_timespec64_to_timespec (*abstime);
>>> +
>>> + return INTERNAL_SYSCALL_CALL (futex, futexp, op, val,
>>> + abstime != NULL ? &ts32 : NULL,
>>> + NULL /* Unused. */,
>>> FUTEX_BITSET_MATCH_ANY); +}
>>> +#endif /* ! __ASSUME_TIME64_SYSCALLS */
>>>
>>> int
>>> __futex_abstimed_wait_cancelable64 (unsigned int* futex_word,
>>> @@ -222,3 +243,28 @@ __futex_clocklock_wait64 (int *futex, int val,
>>> clockid_t clockid,
>>> return -err;
>>> }
>>> +
>>> +int
>>> +__futex_clock_wait_bitset64 (int *futexp, int val, clockid_t
>>> clockid,
>>> + const struct __timespec64 *abstime,
>>> + int private)
>>> +{
>>> + long int relowlevellock-futex.ht;
>> Is there a reason why long int is used instead of int (which is also
>> returned by this function)?
>
> The "long int" type for ret was in the original
> lll_futex_clock_wait_bitset() macro in:
> ./sysdeps/nptl/lowlevellock-futex.h (line 102).
>
> It is also returned when this macro is pasted.
>
> The reason may be that lll_syscall_futex() macro also uses long int to
> get return value from INTERNAL_SYSCALL() (Line 70 in the above file).
>
> Then depending on architecture either long or int are used to get the
> status of the syscall.
>
>
> However, in the end the value returned by lll_futex_clock_wait_bitset()
> is casted to int anyway:
>
> git grep -n "= lll_futex_clock_wait_bitset"
> nptl/pthread_mutex_timedlock.c:271:
> int err = lll_futex_clock_wait_bitset (&mutex->__data.__lock,
>
> sysdeps/nptl/futex-internal.h:288:
> int err = lll_futex_clock_wait_bitset (futex_word, expected,
>
> sysdeps/nptl/futex-internal.h:324:
> int err = lll_futex_clock_wait_bitset (futex_word, expected,
>
> Hence, IMHO it would be correct to change 'long int' to 'int'. Or am I
> wrong?
Yes, I think this is okay.
>
>>> + if (! lll_futex_supported_clockid (clockid))
>>> + {
>>> + return -EINVAL;
>>> + }
>>> +
>>> + const unsigned int clockbit =
>>> + (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME : 0;
>>> + const int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit,
>>> private); +
>>> + ret = INTERNAL_SYSCALL_CALL (futex_time64, futexp, op, val,
>>> + abstime, NULL /* Unused. */,
>>> + FUTEX_BITSET_MATCH_ANY);
>>> +#ifndef __ASSUME_TIME64_SYSCALLS
>>> + if (ret == -ENOSYS)
>>> + ret = __futex_clock_wait_bitset32 (futexp, val, clockid,
>>> abstime, private); +#endif
>>> + return ret;
>>> +}
>>> diff --git a/sysdeps/nptl/futex-internal.h
>>> b/sysdeps/nptl/futex-internal.h index 8a5f62768f..cd356e4fa8 100644
>>> --- a/sysdeps/nptl/futex-internal.h
>>> +++ b/sysdeps/nptl/futex-internal.h
>>> @@ -603,4 +603,9 @@ __futex_clocklock64 (int *futex, clockid_t
>>> clockid, return err;
>>> }
>>>
>>> +int
>>> +__futex_clock_wait_bitset64 (int *futexp, int val, clockid_t
>>> clockid,
>>> + const struct __timespec64 *abstime,
>>> + int private) attribute_hidden;
>>> +
>>> #endif /* futex-internal.h */
>>>
>>
>
>
> 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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-10-21 6:50 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-19 14:57 [PATCH] y2038: nptl: Provide __futex_clock_wait_bitset64 to support 64 bit bitset Lukasz Majewski
2020-10-20 11:01 ` Stefan Liebler via Libc-alpha
2020-10-20 13:31 ` Lukasz Majewski
2020-10-21 6:49 ` Stefan Liebler via Libc-alpha
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).