[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v9 02/20] x86/VPMU: Manage VPMU_CONTEXT_SAVE flag in vpmu_save_force()



On 08/11/2014 09:28 AM, Jan Beulich wrote:
On 08.08.14 at 18:55, <boris.ostrovsky@xxxxxxxxxx> wrote:
vpmu_load() may leave VPMU_CONTEXT_SAVE set after calling vpmu_save_force() if
the context is not loaded.
To me this is ambiguous (and looking at the patch I can't really
resolve it): Do you mean to say this can validly happen, or that it
could have happened in error prior to the patch? In any event
please alter the description to either clearly state what the wrong
old behavior was, or what the correct new behavior is to be.

What I was trying to say (not particularly successfully, apparently) is that currently (i.e. prior to the patch) we may end up not clearing VPMU_CONTEXT_SAVE flag after calling vpmu_save_forced(). And that would be a bug, which this patch is fixing.

(It doesn't help that the diff doesn't show that the flag is cleared right after ->arch_vpmu_save() below)

-boris


Jan

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
---
  xen/arch/x86/hvm/vpmu.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/hvm/vpmu.c b/xen/arch/x86/hvm/vpmu.c
index 63765fa..26e971b 100644
--- a/xen/arch/x86/hvm/vpmu.c
+++ b/xen/arch/x86/hvm/vpmu.c
@@ -130,6 +130,8 @@ static void vpmu_save_force(void *arg)
      if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED) )
          return;
+ vpmu_set(vpmu, VPMU_CONTEXT_SAVE);
+
      if ( vpmu->arch_vpmu_ops )
          (void)vpmu->arch_vpmu_ops->arch_vpmu_save(v);
@@ -178,7 +180,6 @@ void vpmu_load(struct vcpu *v)
           */
          if ( vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED) )
          {
-            vpmu_set(vpmu, VPMU_CONTEXT_SAVE);
              on_selected_cpus(cpumask_of(vpmu->last_pcpu),
                               vpmu_save_force, (void *)v, 1);
              vpmu_reset(vpmu, VPMU_CONTEXT_LOADED);
@@ -195,7 +196,6 @@ void vpmu_load(struct vcpu *v)
          vpmu = vcpu_vpmu(prev);
/* Someone ran here before us */
-        vpmu_set(vpmu, VPMU_CONTEXT_SAVE);
          vpmu_save_force(prev);
          vpmu_reset(vpmu, VPMU_CONTEXT_LOADED);
--
1.8.1.4




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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.