Re: [PATCH] x86/xen: avoid updating TLS descriptors if they haven't changed

[David Vrabel <david.vrabel@citrix.com>]

On 07/06/12 18:01, David Vrabel wrote:
> From: David Vrabel <david.vrabel@citrix.com>
> 
> When switching tasks in a Xen PV guest, avoid updating the TLS
> descriptors if they haven't changed.  This improves the speed of
> context switches by almost 10% as much of the time the descriptors are
> the same or only one is different.
> 
> The descriptors written into the GDT by Xen are modified from the
> values passed in the update_descriptor hypercall so we keep shadow
> copies of the three TLS descriptors to compare against.
> 
> lmbench3 test     Before  After  Improvement
> --------------------------------------------
> lat_ctx -s 32 24   7.19    6.52  9%
> lat_pipe          12.56   11.66  7%
> 
> Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> ---
> I note that the comment in asm/desc_defs.h says the 'a' and 'b' fields
> in desc_struct as deprecated but there seems to be no suitable
> alternatives.

ping?  Any opinion on this patch from the x86 side?  If it's okay can we
get an ack so Konrad can take the patch via his tree.

Thanks.

David

> ---
>  arch/x86/xen/enlighten.c |   30 +++++++++++++++++++++++++++---
>  1 files changed, 27 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index e74df95..18e14af 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -124,6 +124,19 @@ struct shared_info *HYPERVISOR_shared_info = (void *)&xen_dummy_shared_info;
>   */
>  static int have_vcpu_info_placement = 1;
>  
> +struct tls_descs {
> +	struct desc_struct desc[3];
> +};
> +
> +/*
> + * Updating the 3 TLS descriptors in the GDT on every task switch is
> + * surprisingly expensive so we avoid updating them if they haven't
> + * changed.  Since Xen writes different descriptors than the one
> + * passed in the update_descriptor hypercall we keep shadow copies to
> + * compare against.
> + */
> +static DEFINE_PER_CPU(struct tls_descs, shadow_tls_desc);
> +
>  static void clamp_max_cpus(void)
>  {
>  #ifdef CONFIG_SMP
> @@ -535,9 +548,20 @@ static void __init xen_load_gdt_boot(const struct desc_ptr *dtr)
>  static void load_TLS_descriptor(struct thread_struct *t,
>  				unsigned int cpu, unsigned int i)
>  {
> -	struct desc_struct *gdt = get_cpu_gdt_table(cpu);
> -	xmaddr_t maddr = arbitrary_virt_to_machine(&gdt[GDT_ENTRY_TLS_MIN+i]);
> -	struct multicall_space mc = __xen_mc_entry(0);
> +	struct desc_struct *shadow = &per_cpu(shadow_tls_desc, cpu).desc[i];
> +	struct desc_struct *gdt;
> +	xmaddr_t maddr;
> +	struct multicall_space mc;
> +
> +	if (shadow->a == t->tls_array[i].a && shadow->b == t->tls_array[i].b)
> +		return;
> +
> +	shadow->a = t->tls_array[i].a;
> +	shadow->b = t->tls_array[i].b;
> +
> +	gdt = get_cpu_gdt_table(cpu);
> +	maddr = arbitrary_virt_to_machine(&gdt[GDT_ENTRY_TLS_MIN+i]);
> +	mc = __xen_mc_entry(0);
>  
>  	MULTI_update_descriptor(mc.mc, maddr.maddr, t->tls_array[i]);
>  }