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

[Avi Kivity <avi@qumranet.com>]

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.


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/