Re: [PATCH v2 01/28] kernel: Standardize vdso_datapage

[Thomas Gleixner <tglx@linutronix.de>]

On Thu, 29 Nov 2018, Vincenzo Frascino wrote:
> +/*
> + * Copyright (C) 2012 ARM Limited
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.

Please use SPDX identifier for the license and get rid of the boiler plate.

> +#include <linux/types.h>
> +
> +struct vdso_data {
> +	__u64 cs_cycle_last;	/* Timebase at clocksource init */

Why do you want to use the __u* variants? This is not a header exposed to
user space. It's part of the kernel.

> +	__u64 raw_time_sec;	/* Raw time */
> +	__u64 raw_time_nsec;
> +	__u64 xtime_clock_sec;	/* Kernel time */
> +	__u64 xtime_clock_nsec;
> +	__u64 xtime_coarse_sec;	/* Coarse time */
> +	__u64 xtime_coarse_nsec;
> +	__u64 wtm_clock_sec;	/* Wall to monotonic time */
> +	__u64 wtm_clock_nsec;
> +	__u32 tb_seq_count;	/* Timebase sequence counter */
> +	__u32 cs_mono_mult;	/* NTP-adjusted clocksource multiplier */
> +	__u32 cs_shift;		/* Clocksource shift (mono = raw) */
> +	__u32 cs_raw_mult;	/* Raw clocksource multiplier */
> +	__u32 tz_minuteswest;	/* Whacky timezone stuff */
> +	__u32 tz_dsttime;
> +	__u32 use_syscall;

This struct is also suboptimal cache line and access pattern wise. Aside of
that please look at the x86 variant of this struct. It's optimized and
handles all the clock variants without adding randomly named struct
members.

Thanks,

	tglx