Hi Catalin,
On 15/04/2019 18:35, Catalin Marinas wrote:
> On Mon, Apr 01, 2019 at 12:51:48PM +0100, Vincenzo Frascino wrote:
>> diff --git a/arch/arm64/kernel/vdso.c b/arch/arm64/kernel/vdso.c
>> index 2d419006ad43..47ba72345739 100644
>> --- a/arch/arm64/kernel/vdso.c
>> +++ b/arch/arm64/kernel/vdso.c
>> @@ -245,6 +245,8 @@ void update_vsyscall(struct timekeeper *tk)
>> vdso_data->cs_shift = tk->tkr_mono.shift;
>> }
>>
>> + vdso_data->hrtimer_res = hrtimer_resolution;
>> +
>> smp_wmb();
>> ++vdso_data->tb_seq_count;
>> }
>> diff --git a/arch/arm64/kernel/vdso/gettimeofday.S b/arch/arm64/kernel/vdso/gettimeofday.S
>> index c39872a7b03c..7a2cd2f8e13a 100644
>> --- a/arch/arm64/kernel/vdso/gettimeofday.S
>> +++ b/arch/arm64/kernel/vdso/gettimeofday.S
>> @@ -296,32 +296,35 @@ ENDPROC(__kernel_clock_gettime)
>> /* int __kernel_clock_getres(clockid_t clock_id, struct timespec *res); */
>> ENTRY(__kernel_clock_getres)
>> .cfi_startproc
>> + adr vdso_data, _vdso_data
>> cmp w0, #CLOCK_REALTIME
>> ccmp w0, #CLOCK_MONOTONIC, #0x4, ne
>> ccmp w0, #CLOCK_MONOTONIC_RAW, #0x4, ne
>> - b.ne 1f
>> + b.ne 2f
>>
>> - ldr x2, 5f
>> - b 2f
>> -1:
>> +1: /* Get hrtimer_res */
>> + seqcnt_acquire
>> + syscall_check fail=5f
>> + ldr x2, [vdso_data, #CLOCK_REALTIME_RES]
>> + seqcnt_check fail=1b
>> + b 3f
>> +2:
>
> We talked briefly but I'm still confused why we need the fallback to the
> syscall here if archdata.vdso_direct is false. Is it because if the
> timer driver code sets vdso_direct to false, we don't don't support
> highres timers? If my understanding is correct, you may want to move the
> hrtimer_res setting in update_vsyscall() to the !use_syscall block.
>
Ok, so let me try to provide more details on what I mentioned yesterday:
- clock_getres syscall follows the rules of what defined in posix-timers.c
- based on the clock_id that, for this purpose, can be separated in coarse and
non-coarse calls either posix_get_coarse_res() or posix_get_hrtimer_res().
- if clock id is set to a coarse clock and posix_get_coarse_res() is invoked,
happens what follows:
static int posix_get_coarse_res(const clockid_t which_clock,
struct timespec64 *tp)
{
*tp = ktime_to_timespec64(KTIME_LOW_RES);
return 0;
}
Note that since CONFIG_1HZ seems not supported (jiffies.h) by the kernel in this
case we do not need rounding in our vDSO implementation.
- if clock id is set to non-coarse and posix_get_hrtimer_res() is invoked,
happens the following:
static int posix_get_hrtimer_res(clockid_t which_clock, struct timespec64 *tp)
{
tp->tv_sec = 0;
tp->tv_nsec = hrtimer_resolution;
return 0;
}
hrtimer_resolution can be high res or low res depending on the call of
hrtimer_switch_to_hres(). For us the only way to preserve the correct value is
to keep it in the vdso data page.
- The assembly code mimics exactly the same behaviour detailed above, with one
difference: the one related to the use_syscall parameter which is specific to arm64.
The use_syscall parameter is set by arm_arch_timer and consumed by
update_vsyscall(). To mirror what update_vsyscall does in update_vsyscall() I
check "syscall_check fail=5f" in clock_getres vdso function.
Said that, even if functionally it is the same thing, I think it is logically
more correct to have hrtimer_res setting inside the !use_syscall block, hence I
am going to change it in the next iteration.
Please let me know your thoughts.
--
Regards,
Vincenzo
