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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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.

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