Re: [PATCH 1/3] lib/vdso: Delay mask application in do_hres()

Vincenzo Frascino
2019-06-26T09:25:27Z

Hi Thomas,

On 26/06/2019 07:38, Thomas Gleixner wrote:
> On Tue, 25 Jun 2019, Thomas Gleixner wrote:
>> On Tue, 25 Jun 2019, Vincenzo Frascino wrote:
>>> do_hres() in the vDSO generic library masks the hw counter value
>>> immediately after reading it.
>>>
>>> Postpone the mask application after checking if the syscall fallback is
>>> enabled, in order to be able to detect a possible fallback for the
>>> architectures that have masks smaller than ULLONG_MAX.
>>
>> Right. This only worked on x86 because the mask is there ULLONG_MAX for all
>> VDSO capable clocksources, i.e. that ever worked just by chance.
>
> But it's actually worse than that:
>
>>> + cycles &= vd->mask;
>>> if (cycles > last)
>>> ns += (cycles - last) * vd->mult;
>>> ns >>= vd->shift;
>
> This is broken for any clocksource which can legitimately wrap around. The
> core timekeeping does the right thing:
>
> (cycles - last) & mask
>
> That makes sure that a wraparound is correctly handled. With the above the
> wrap around would be ignored due to
>
> if (cycles > last)
>

You are right. Thanks for spotting it.

...

--
Regards,
Vincenzo