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: Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] x86/hvm/pmtimer: improving scalability of virtual time update on Xen 4.0.0
From: Song Xiang <classicxsong@xxxxxxxxx>
Date: Wed, 17 Nov 2010 23:13:43 +0000
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Wed, 17 Nov 2010 07:40:39 -0800
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:cc:message-id:from:to :in-reply-to:content-type:content-transfer-encoding:mime-version :subject:date:references:x-mailer; bh=7ZrnflKCDSSZScXtm2DmIzQ6kQkF5Gqw+BNohqoJhnM=; b=Cr03H/pZ5gJokuvGCE9o99e5uC+gI6ylTYCru9+ihZ3bLCVgP3o667t4Q447qZrYtW KjrCe6wX5ts5yJQfN5Gq8aDbvBpg/SMUyIXMgVoze+SF7WxZEbALL6VewpPx5a3LtvHK 6FzwoBWDTjgRLB1K85z9xeN6QOL9WQChwcj2A=
Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=cc:message-id:from:to:in-reply-to:content-type :content-transfer-encoding:mime-version:subject:date:references :x-mailer; b=H1+XFcnn/vF+MV+wHZUDuLvH53jYYr4E0Zh4FsZNtIwVfrXIw/voHf7uhk6EGHuiRb /zQBEoYcMz8mLpssemUMym76zUyIdTVpFwUju/iWd2yt7VytdAXbzRGbxOOs9E9PaEHh nISSbRzfOI26pvaHpHzd/aGjhUtBM+dIV1U+Y=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20101117144745.GG21112@xxxxxxxxxxxxxxxxxxxxxxx>
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> <20101117144745.GG21112@xxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
It can be as follows:
@@ -90,8 +90,8 @@ 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;
-    s->pm.tmr_val &= TMR_VAL_MASK;
+    *(volatile uint32_t *)&s->pm.tmr_val = (s->pm.tmr_val +
+ (((curr_gtime - s->last_gtime) * s->scale) >> 32)) & TMR_VAL_MASK;
     s->last_gtime = curr_gtime;

     /* If the counter's MSB has changed, set the status bit */


but it seems gcc (gcc 4.4.4) has omitted the statement s->pm.tmr_val &= TMR_VAL_MASK;
in the object file

在 2010-11-17,下午2:47, Tim Deegan 写道:

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