Re: [PATCH 2/4] [PATCH] cacheline-align kvmclock structures

Avi Kivity
2008-03-06T06:45:37Z

Glauber Costa wrote:
> Align the kvm_vcpu_time array to the size of a cacheline.
>
> Signed-off-by: Glauber Costa <gcosta@redhat.com>
> ---
> arch/x86/kernel/kvmclock.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
> index b8da3bf..d82406a 100644
> --- a/arch/x86/kernel/kvmclock.c
> +++ b/arch/x86/kernel/kvmclock.c
> @@ -36,7 +36,7 @@ early_param("no-kvmclock", parse_no_kvmc
> struct shared_info shared_info __attribute__((__aligned__(PAGE_SIZE)));
>
> /* The hypervisor will put information about time periodically here */
> -static struct kvm_vcpu_time_info hv_clock[NR_CPUS];
> +static struct kvm_vcpu_time_info hv_clock[NR_CPUS] __cacheline_aligned;
> #define get_clock(cpu, field) hv_clock[cpu].field
>
> static inline u64 kvm_get_delta(u64 last_tsc)
>

I think this will align only the array itself, not members, so any write
will (and the following reads) will cause a cacheline bounce.

Switching to per_cpu() both clarifies the intent and fixes the issue.
Still need the 8-byte alignment. Might be best to stick that on the
structure declaration, so it applies to all guests.

--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.