WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] [PATCH] x86/hvm/pmtimer: improving scalability of virtua

To: Song Xiang <classicxsong@xxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] x86/hvm/pmtimer: improving scalability of virtual time update on Xen 4.0.0
From: Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Date: Wed, 17 Nov 2010 14:47:45 +0000
Cc: Haibo Chen <oldseawave@xxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, Keir Fraser <keir@xxxxxxx>, Jan Beulich <JBeulich@xxxxxxxxxx>
Delivery-date: Wed, 17 Nov 2010 06:50:10 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <12AF73E6-A34C-46BC-BFC1-2B7993AAF069@xxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <12AF73E6-A34C-46BC-BFC1-2B7993AAF069@xxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mutt/1.5.20 (2009-06-14)
At 21:39 +0000 on 17 Nov (1290029945), Song Xiang wrote:
> diff --git a/xen/arch/x86/hvm/pmtimer.c b/xen/arch/x86/hvm/pmtimer.c
> index 48fe26a..587944f 100644
> --- a/xen/arch/x86/hvm/pmtimer.c
> +++ b/xen/arch/x86/hvm/pmtimer.c
> @@ -90,7 +90,7 @@ static void pmt_update_time(PMTState *s)
> 
>      /* Update the timer */
>      curr_gtime = hvm_get_guest_time(s->vcpu);
> -    s->pm.tmr_val += ((curr_gtime - s->last_gtime) * s->scale) >> 32;
> +    *(volatile uint32_t *)&s->pm.tmr_val = s->pm.tmr_val + (((curr_gtime - 
> s->last_gtime) * s->scale) >> 32);
>      s->pm.tmr_val &= TMR_VAL_MASK;
>      s->last_gtime = curr_gtime;

That doesn't make it an atomic update!  The variable is still written
to twice. :)  You need to calculate the new tmr_val in a scratch
variable, and then write it back once at the end of the function.
(And no 'volatile' wll be necessary).

Cheers,

Tim.

> @@ -206,10 +206,19 @@ static int handle_pmt_io(
> 
>      if ( dir == IOREQ_READ )
>      {
> -        spin_lock(&s->lock);
> -        pmt_update_time(s);
> -        *val = s->pm.tmr_val;
> -        spin_unlock(&s->lock);
> +        /*
> +         * if acquired the PMTState lock then update the time
> +         * else other vcpu is updating it ,it should be up to date.
> +         */
> +        if (spin_trylock(&s->lock)) {
> +            pmt_update_time(s);
> +            *val = s->pm.tmr_val;
> +            spin_unlock(&s->lock);
> +        }
> +        else {
> +            spin_barrier(&s->lock);
> +            *val = s->pm.tmr_val;
> +        }
>          return X86EMUL_OKAY;
>      }
> 

> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel


-- 
Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel