Hi Thomas, thank you for reviewing my patches. On 29/11/2018 22:39, Thomas Gleixner wrote: > 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. > Agreed, I will fix it in the next iteration. >> +#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. > This patch contains the same datatypes of the file from which is copied. Said that, I agree with you and I will fix this in v3. >> + __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. > I had a look at the x86 implementation and I agree with you and Arnd that it is better than what I am proposing here. Therefore, in my v3, I will follow that model for the vdso unification. Thanks to both for pointing that out and for providing the rationale behind it. > Thanks, > > tglx > -- Regards, Vincenzo
