Re: [PATCH] y2038: replace timespec with timespec64 in clock_settime

[Jan Kiszka <jan.kiszka@web.de>]

On 29.09.20 08:27, chensong wrote:
>
>
> On 2020年09月29日 00:52, Jan Kiszka wrote:
>> On 23.09.20 03:40, song wrote:
>>>
>>>
>>> On 2020/9/22 下午11:16, Jan Kiszka wrote:
>>>> On 21.09.20 14:32, chensong wrote:
>>>>> Upstream has used timespec64 to replace timespec inside kernel and
>>>>> used __kernel_timespec to replace timespect in syscalls, therefore
>>>>> we must keep aligned with upstream.
>>>>>
>>>>> clock_settime is a point to get started at, it involves 2 parts:
>>>>> 1, syscall in ./include/trace/events/cobalt-posix.h for 64bits
>>>>> 2, compat syscall in kernel/cobalt/posix/syscall32.c for 32bits
>>>>>
>>>>> some new functions are implemented to keep the compatibility with
>>>>> other syscalls which haven't been handled yet and will be removed
>>>>> in the end of the work.
>>>>
>>>> So, this switches also existing syscalls to 64-bit types, breaking the
>>>> current ABI, no? How much effort would it be to keep the old ABI,
>>>> adding a 64-bit one aside of it?
>>>
>>> Not breaking current ABI, i didn't add any new ABIs specific for 64bits,
>>> i added some new static functions, like ts2ns and ts2ns64,
>>> sys32_put_timespec and sys64_put_timespec, specific for clock_settime.
>>>
>>
>> clock_settime accepts struct timespec so far, you are changing that to
>> struct __kernel_timespec. If I look at its definition in the kernel, it
>> uses __kernel_time64_t as tv_sec type. But our current 32-bit users
>> still expects a 32-bit type here, no? Would that change already require
>> a new clock_settime64 syscall if we wanted to stay binary compatible?
>
> 32 bit user space processes go to
> COBALT_SYSCALL32emu(clock_settime,current,(clockid_t clock_id,const
> struct compat_timespec __user *u_ts)) defined in
> ./kernel/cobalt/posix/syscall32.c. It uses compat_timespec which 32it is
> compatible with.
>
> I tested 32bit application and 64bit application on 64bit system
> respeactively
> 32bit process -- clock_settime -- COBALT_SYSCALL32emu(clock_settime
> 64bit process -- clock_settime -- COBALT_SYSCALL(clock_settime)
>

You also need to consider 32-bit kernels, not on x86 anymore (i386 is
practically dead), but e.g. ARMv7. There we would have a breakage now,
don't we?

Jan