All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] x86/time: slightly streamline __update_vcpu_system_time()
@ 2015-10-13 12:27 Jan Beulich
  2015-10-13 13:22 ` Andrew Cooper
  2015-10-13 13:39 ` Andrew Cooper
  0 siblings, 2 replies; 3+ messages in thread
From: Jan Beulich @ 2015-10-13 12:27 UTC (permalink / raw)
  To: xen-devel; +Cc: Andrew Cooper, Keir Fraser

[-- Attachment #1: Type: text/plain, Size: 1891 bytes --]

Fold two if()-s using the same condition, converting the memset() so
far separating them to a simple initializer. Move common assignments
out of the conditional. Drop an unnecessary initializer.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Additionally the is_hvm_domain() visible in the last hunk's tail
context looks, along with a few other releated ones, suspicious -
shouldn't these be has_hvm_container_domain() instead?

--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -790,9 +790,9 @@ uint64_t tsc_ticks2ns(uint64_t ticks)
 static void __update_vcpu_system_time(struct vcpu *v, int force)
 {
     struct cpu_time       *t;
-    struct vcpu_time_info *u, _u;
+    struct vcpu_time_info *u, _u = {};
     struct domain *d = v->domain;
-    s_time_t tsc_stamp = 0;
+    s_time_t tsc_stamp;
 
     if ( v->vcpu_info == NULL )
         return;
@@ -816,28 +816,21 @@ static void __update_vcpu_system_time(st
         }
         else
             tsc_stamp = gtime_to_gtsc(d, stime);
-    }
-    else
-    {
-        tsc_stamp = t->local_tsc_stamp;
-    }
-
-    memset(&_u, 0, sizeof(_u));
 
-    if ( d->arch.vtsc )
-    {
-        _u.tsc_timestamp     = tsc_stamp;
-        _u.system_time       = t->stime_local_stamp;
         _u.tsc_to_system_mul = d->arch.vtsc_to_ns.mul_frac;
         _u.tsc_shift         = d->arch.vtsc_to_ns.shift;
     }
     else
     {
-        _u.tsc_timestamp     = t->local_tsc_stamp;
-        _u.system_time       = t->stime_local_stamp;
+        tsc_stamp = t->local_tsc_stamp;
+
         _u.tsc_to_system_mul = t->tsc_scale.mul_frac;
         _u.tsc_shift         = (s8)t->tsc_scale.shift;
     }
+
+    _u.tsc_timestamp = tsc_stamp;
+    _u.system_time   = t->stime_local_stamp;
+
     if ( is_hvm_domain(d) )
         _u.tsc_timestamp += v->arch.hvm_vcpu.cache_tsc_offset;
 




[-- Attachment #2: x86-streamline-uvst.patch --]
[-- Type: text/plain, Size: 1946 bytes --]

x86/time: slightly streamline __update_vcpu_system_time()

Fold two if()-s using the same condition, converting the memset() so
far separating them to a simple initializer. Move common assignments
out of the conditional. Drop an unnecessary initializer.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Additionally the is_hvm_domain() visible in the last hunk's tail
context looks, along with a few other releated ones, suspicious -
shouldn't these be has_hvm_container_domain() instead?

--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -790,9 +790,9 @@ uint64_t tsc_ticks2ns(uint64_t ticks)
 static void __update_vcpu_system_time(struct vcpu *v, int force)
 {
     struct cpu_time       *t;
-    struct vcpu_time_info *u, _u;
+    struct vcpu_time_info *u, _u = {};
     struct domain *d = v->domain;
-    s_time_t tsc_stamp = 0;
+    s_time_t tsc_stamp;
 
     if ( v->vcpu_info == NULL )
         return;
@@ -816,28 +816,21 @@ static void __update_vcpu_system_time(st
         }
         else
             tsc_stamp = gtime_to_gtsc(d, stime);
-    }
-    else
-    {
-        tsc_stamp = t->local_tsc_stamp;
-    }
-
-    memset(&_u, 0, sizeof(_u));
 
-    if ( d->arch.vtsc )
-    {
-        _u.tsc_timestamp     = tsc_stamp;
-        _u.system_time       = t->stime_local_stamp;
         _u.tsc_to_system_mul = d->arch.vtsc_to_ns.mul_frac;
         _u.tsc_shift         = d->arch.vtsc_to_ns.shift;
     }
     else
     {
-        _u.tsc_timestamp     = t->local_tsc_stamp;
-        _u.system_time       = t->stime_local_stamp;
+        tsc_stamp = t->local_tsc_stamp;
+
         _u.tsc_to_system_mul = t->tsc_scale.mul_frac;
         _u.tsc_shift         = (s8)t->tsc_scale.shift;
     }
+
+    _u.tsc_timestamp = tsc_stamp;
+    _u.system_time   = t->stime_local_stamp;
+
     if ( is_hvm_domain(d) )
         _u.tsc_timestamp += v->arch.hvm_vcpu.cache_tsc_offset;
 

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] x86/time: slightly streamline __update_vcpu_system_time()
  2015-10-13 12:27 [PATCH] x86/time: slightly streamline __update_vcpu_system_time() Jan Beulich
@ 2015-10-13 13:22 ` Andrew Cooper
  2015-10-13 13:39 ` Andrew Cooper
  1 sibling, 0 replies; 3+ messages in thread
From: Andrew Cooper @ 2015-10-13 13:22 UTC (permalink / raw)
  To: Jan Beulich, xen-devel; +Cc: Keir Fraser

On 13/10/15 13:27, Jan Beulich wrote:
> Fold two if()-s using the same condition, converting the memset() so
> far separating them to a simple initializer. Move common assignments
> out of the conditional. Drop an unnecessary initializer.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] x86/time: slightly streamline __update_vcpu_system_time()
  2015-10-13 12:27 [PATCH] x86/time: slightly streamline __update_vcpu_system_time() Jan Beulich
  2015-10-13 13:22 ` Andrew Cooper
@ 2015-10-13 13:39 ` Andrew Cooper
  1 sibling, 0 replies; 3+ messages in thread
From: Andrew Cooper @ 2015-10-13 13:39 UTC (permalink / raw)
  To: Jan Beulich, xen-devel; +Cc: Keir Fraser

On 13/10/15 13:27, Jan Beulich wrote:
> Fold two if()-s using the same condition, converting the memset() so
> far separating them to a simple initializer. Move common assignments
> out of the conditional. Drop an unnecessary initializer.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> Additionally the is_hvm_domain() visible in the last hunk's tail
> context looks, along with a few other releated ones, suspicious -
> shouldn't these be has_hvm_container_domain() instead?

I believe you are correct.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2015-10-13 13:39 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-13 12:27 [PATCH] x86/time: slightly streamline __update_vcpu_system_time() Jan Beulich
2015-10-13 13:22 ` Andrew Cooper
2015-10-13 13:39 ` Andrew Cooper

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.