[PATCH v4] y2038: Convert aio_suspend to support 64 bit time

[Adhemerval Zanella]

On 27/11/2020 11:17, Lukasz Majewski wrote:
> 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?

I haven't changed the futex_reltimed_wait{_cancelable} because they currently
do not return EOVERFLOW.  The aio_misc.h code will possible return with
this change.