Re: [PATCH v5 01/23] kernel: Standardize vdso_datapage

[Vincenzo Frascino <vincenzo.frascino@arm.com>]

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel