Re: [Xen-devel] [PATCH] vpmu intel: move setting alloc flag

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.03.13 at 14:44, Dietmar Hahn <dietmar.hahn@xxxxxxxxxxxxxx> wrote:
> This simple patch moves the setting of the ALLOCATED flag to where the
> allocation gets done.

While not wrong in any way, isn't the placement before this patch
slightly more logical, namely because of ...

> Signed-off-by: Dietmar Hahn <dietmar.hahn@xxxxxxxxxxxxxx>
> 
> 
> diff -r a6b81234b189 xen/arch/x86/hvm/vmx/vpmu_core2.c
> --- a/xen/arch/x86/hvm/vmx/vpmu_core2.c Mon Mar 11 16:13:42 2013 +0000
> +++ b/xen/arch/x86/hvm/vmx/vpmu_core2.c Tue Mar 12 14:30:37 2013 +0100
> @@ -346,6 +346,7 @@ static int core2_vpmu_alloc_resource(str
>          goto out2;
>      core2_vpmu_cxt->pmu_enable = pmu_enable;
>      vpmu->context = (void *)core2_vpmu_cxt;
> +    vpmu_set(vpmu, VPMU_CONTEXT_ALLOCATED);
>  
>      return 1;
>   out2:
> @@ -384,7 +385,6 @@ static int core2_vpmu_msr_common_check(u

... the check here:

    if ( unlikely(!vpmu_is_set(vpmu, VPMU_CONTEXT_ALLOCATED)) &&

>          (vpmu->context != NULL ||
>           !core2_vpmu_alloc_resource(current)) )
>          return 0;

Or as another consistent alternative, I'd suggest to move the two
other portions of the if() clause also into that function.

Jan

> -    vpmu_set(vpmu, VPMU_CONTEXT_ALLOCATED);
>  
>      /* Do the lazy load staff. */
>      if ( !vpmu_is_set(vpmu, VPMU_CONTEXT_LOADED) )
> 
> 
> -- 
> Company details: http://ts.fujitsu.com/imprint.html 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx 
> http://lists.xen.org/xen-devel 




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