On 23/02/2019 16:51, Thomas Gleixner wrote:
> On Fri, 22 Feb 2019, Vincenzo Frascino wrote:
>> +/*
>> + * There is one vdso_clocksource object in vvar for each vDSO clocksource
>> + * (mono, raw). This struct is designed to keep vdso_data "cache-line friendly"
>> + * and optimal in terms of access pattern.
>> + *
>> + * Note that mask and shift are the same for mono and raw.
>> + */
>> +struct vdso_clocksource {
>> + u64 mask; /* Clocksource mask */
>> + u32 mult; /* Clocksource multiplier */
>> + u32 shift; /* Clocksource shift */
>
> Can you please get rid of the tail comments and use proper kerneldoc
> format?
>
I will fix it in v6, thanks.
>> +/*
>> + * vdso_data will be accessed by 32 and 64 bit code at the same time
>> + * so we should be careful before modifying this structure.
>> + */
>> +struct vdso_data {
>> + u32 seq; /* Timebase sequence counter */
>> +
>> + s32 clock_mode;
>> + u64 cycle_last; /* Timebase at clocksource init */
>> +
>> + struct vdso_clocksource cs[CLOCKSOURCE_BASES];
>
> Why would you need different clocksource parameters? That really bloats the
> data structure and makes the cache access pattern worse. Also the vdso
> update needs to copy the same data over and over for no value.
>
> The only clock ID which needs a different mult/shift would be
> MONOTONIC_RAW, but if we expose that through the VDSO then we really can be
> smarter than this. See incomplete and uncompilable patch below for
> reference. You get the idea.
>
Ok, thank you for providing the reference code. I will update my patches in v6
accordingly.
...
--
Regards,
Vincenzo
