On 11/01/2012 02:47 AM, Marcelo Tosatti wrote:
> Originally from Jeremy Fitzhardinge.
> 
> Introduce generic, non hypervisor specific, pvclock initialization 
> routines.
> 
> Signed-off-by: Marcelo Tosatti <mtosa...@redhat.com>
> 
> Index: vsyscall/arch/x86/kernel/pvclock.c
> ===================================================================
> --- vsyscall.orig/arch/x86/kernel/pvclock.c
> +++ vsyscall/arch/x86/kernel/pvclock.c
> @@ -17,6 +17,10 @@
>  
>  #include <linux/kernel.h>
>  #include <linux/percpu.h>
> +#include <linux/notifier.h>
> +#include <linux/sched.h>
> +#include <linux/gfp.h>
> +#include <linux/bootmem.h>
>  #include <asm/pvclock.h>
>  
>  static u8 valid_flags __read_mostly = 0;
> @@ -122,3 +126,67 @@ void pvclock_read_wallclock(struct pvclo
>  
>       set_normalized_timespec(ts, now.tv_sec, now.tv_nsec);
>  }
> +
> +static aligned_pvti_t *pvclock_vdso_info;
is there a non-aligned pvti ?

I would also add that "pvti" is not exactly the most descriptive name
I've seen.

> +
> +static struct pvclock_vsyscall_time_info *
> +pvclock_get_vsyscall_user_time_info(int cpu)
> +{
> +     if (pvclock_vdso_info == NULL) {
> +             BUG();
> +             return NULL;
> +     }
> +
> +     return &pvclock_vdso_info[cpu].info;
> +}
> +
if (!pvclock_vdso_info)
    BUG();
return &pvclock...

> +struct pvclock_vcpu_time_info *pvclock_get_vsyscall_time_info(int cpu)
> +{
> +     return &pvclock_get_vsyscall_user_time_info(cpu)->pvti;
> +}
> +
> +int pvclock_task_migrate(struct notifier_block *nb, unsigned long l, void *v)
> +{
> +     struct task_migration_notifier *mn = v;
> +     struct pvclock_vsyscall_time_info *pvti;
> +
> +     pvti = pvclock_get_vsyscall_user_time_info(mn->from_cpu);
> +
> +     if (pvti == NULL)
> +             return NOTIFY_DONE;
> +

When is it NULL? IIUC, this is when the vsyscall is disabled, right?
Would you mind adding comments for it?

Also, this is supposed to be an unlikely branch, right?

> +     pvti->migrate_count++;
> +
> +     return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block pvclock_migrate = {
> +     .notifier_call = pvclock_task_migrate,
> +};
> +
> +/*
> + * Initialize the generic pvclock vsyscall state.  This will allocate
> + * a/some page(s) for the per-vcpu pvclock information, set up a
> + * fixmap mapping for the page(s)
> + */
> +int __init pvclock_init_vsyscall(void)
> +{
> +     int idx;
> +     unsigned int size = PVCLOCK_VSYSCALL_NR_PAGES*PAGE_SIZE;
> +
> +     pvclock_vdso_info = __alloc_bootmem(size, PAGE_SIZE, 0);

This will be the most critical path for reading pvclock's time data. So
why can't we:
1) Make it per-node.
2) Of course, as a consequence, make sure all info structures in the
same page are in the same node ?



> +     if (!pvclock_vdso_info)
> +             return -ENOMEM;
> +
> +     memset(pvclock_vdso_info, 0, size);
> +
> +     for (idx = 0; idx <= (PVCLOCK_FIXMAP_END-PVCLOCK_FIXMAP_BEGIN); idx++) {
> +             __set_fixmap(PVCLOCK_FIXMAP_BEGIN + idx,
> +                          __pa_symbol(pvclock_vdso_info) + (idx*PAGE_SIZE),
> +                          PAGE_KERNEL_VVAR);
> +     }
> +
> +     register_task_migration_notifier(&pvclock_migrate);
> +
> +     return 0;
> +}
> Index: vsyscall/arch/x86/include/asm/fixmap.h
> ===================================================================
> --- vsyscall.orig/arch/x86/include/asm/fixmap.h
> +++ vsyscall/arch/x86/include/asm/fixmap.h
> @@ -19,6 +19,7 @@
>  #include <asm/acpi.h>
>  #include <asm/apicdef.h>
>  #include <asm/page.h>
> +#include <asm/pvclock.h>
>  #ifdef CONFIG_X86_32
>  #include <linux/threads.h>
>  #include <asm/kmap_types.h>
> @@ -81,6 +82,10 @@ enum fixed_addresses {
>       VVAR_PAGE,
>       VSYSCALL_HPET,
>  #endif
> +#ifdef CONFIG_PARAVIRT_CLOCK
> +     PVCLOCK_FIXMAP_BEGIN,
> +     PVCLOCK_FIXMAP_END = PVCLOCK_FIXMAP_BEGIN+PVCLOCK_VSYSCALL_NR_PAGES-1,
> +#endif
>       FIX_DBGP_BASE,
>       FIX_EARLYCON_MEM_BASE,
>  #ifdef CONFIG_PROVIDE_OHCI1394_DMA_INIT
> Index: vsyscall/arch/x86/include/asm/pvclock.h
> ===================================================================
> --- vsyscall.orig/arch/x86/include/asm/pvclock.h
> +++ vsyscall/arch/x86/include/asm/pvclock.h
> @@ -13,6 +13,8 @@ void pvclock_read_wallclock(struct pvclo
>                           struct pvclock_vcpu_time_info *vcpu,
>                           struct timespec *ts);
>  void pvclock_resume(void);
> +int __init pvclock_init_vsyscall(void);
> +struct pvclock_vcpu_time_info *pvclock_get_vsyscall_time_info(int cpu);
>  
>  /*
>   * Scale a 64-bit delta by scaling and multiplying by a 32-bit fraction,
> @@ -85,4 +87,17 @@ unsigned __pvclock_read_cycles(const str
>       return version;
>  }
>  
> +struct pvclock_vsyscall_time_info {
> +     struct pvclock_vcpu_time_info pvti;
> +     u32 migrate_count;
> +};
> +
> +typedef union {
> +     struct pvclock_vsyscall_time_info info;
> +     char pad[SMP_CACHE_BYTES];
> +} aligned_pvti_t ____cacheline_aligned;

Please, just align pvclock_vsyscall_time_info. It is 10 thousand times
more descriptive.

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to