Re: [PATCH v7 3/4] linux-user: Support futex_time64

[Laurent Vivier]

Le 13/03/2020 à 23:13, Alistair Francis a écrit :
> On Fri, Mar 13, 2020 at 3:12 PM Alistair Francis <address@hidden> wrote:
>>
>> On Fri, Mar 13, 2020 at 1:14 AM Laurent Vivier <address@hidden> wrote:
>>>
>>> Le 12/03/2020 à 23:13, Alistair Francis a écrit :
>>>> Add support for host and target futex_time64. If futex_time64 exists on
>>>> the host we try that first before falling back to the standard futux
>>>> syscall.
>>>>
>>>> Signed-off-by: Alistair Francis <address@hidden>
>>>> ---
>>>>  linux-user/syscall.c | 144 +++++++++++++++++++++++++++++++++++++++----
>>>>  1 file changed, 131 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>>>> index 60fd775d9c..9ae7a05e38 100644
>>>> --- a/linux-user/syscall.c
>>>> +++ b/linux-user/syscall.c
>>> ...
>>>> @@ -6890,6 +6905,55 @@ static inline abi_long host_to_target_statx(struct 
>>>> target_statx *host_stx,
>>>>  }
>>>>  #endif
>>>>
>>>> +static int do_sys_futex(int *uaddr, int op, int val,
>>>> +                         const struct timespec *timeout, int *uaddr2,
>>>> +                         int val3)
>>>> +{
>>>> +#if HOST_LONG_BITS == 64
>>>> +#if defined(__NR_futex)
>>>> +    /* always a 64-bit time_t, it doesn't define _time64 version  */
>>>> +    return sys_futex(uaddr, op, val, timeout, uaddr2, val3);
>>>> +
>>>> +#endif
>>>> +#else /* HOST_LONG_BITS == 64 */
>>>> +#if defined(__NR_futex_time64)
>>>> +    if (sizeof(timeout->tv_sec) == 8) {
>>>> +        /* _time64 function on 32bit arch */
>>>> +        return sys_futex_time64(uaddr, op, val, timeout, uaddr2, val3);
>>>> +    }
>>>> +#endif
>>>> +#if defined(__NR_futex)
>>>> +    /* old function on 32bit arch */
>>>> +    return sys_futex(uaddr, op, val, timeout, uaddr2, val3);
>>>> +#endif
>>>> +#endif /* HOST_LONG_BITS == 64 */
>>>> +    return -TARGET_ENOSYS;
>>>
>>> You cannot return -TARGET_ENOSYS because return value is supposed to be
>>> a host value as you have direct value from sys_futex().
>>>
>>> ...
>>>
>>>> @@ -7505,7 +7619,7 @@ static abi_long do_syscall1(void *cpu_env, int num, 
>>>> abi_long arg1,
>>>>              ts = cpu->opaque;
>>>>              if (ts->child_tidptr) {
>>>>                  put_user_u32(0, ts->child_tidptr);
>>>> -                sys_futex(g2h(ts->child_tidptr), FUTEX_WAKE, INT_MAX,
>>>> +                do_sys_futex(g2h(ts->child_tidptr), FUTEX_WAKE, INT_MAX,
>>>>                            NULL, NULL, 0);
>>>
>>> And return value is ignored. Anyway at this point we can't do anything
>>> if it doesn't work, but it should not happen. So I think the best to do
>>> is to add a g_assert_not_reached() in place of "return -TARGET_ENOSYS"
>>> in do_sys_futex() (or "#error" if no futex function is available).
>>
>> It sounds like you applied this series, so I'm not going to fix this
>> up. Let me know if you would like me to.
> 
> Ha, I read the wrong cover letter.
> 
> I'll send a new version of this.

I have applied all the series except this patch.

Thanks,
Laurent