From mboxrd@z Thu Jan  1 00:00:00 1970
From: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Subject: Re: [PATCH v2 1/2] xenoprof: fix up ability to disable
	it
Date: Tue, 9 Feb 2016 08:54:53 -0500
Message-ID: <56B9EFAD.3070705@oracle.com>
References: <1454991321-399-1-git-send-email-cardoe@cardoe.com>
	<56B9B9E3.9000700@citrix.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <56B9B9E3.9000700@citrix.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Andrew Cooper <andrew.cooper3@citrix.com>, Doug Goldstein <cardoe@cardoe.com>, xen-devel@lists.xen.org
Cc: Kevin Tian <kevin.tian@intel.com>, Keir Fraser <keir@xen.org>, Jan Beulich <jbeulich@suse.com>, Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>, Jun Nakajima <jun.nakajima@intel.com>, Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
List-Id: xen-devel@lists.xenproject.org

On 02/09/2016 05:05 AM, Andrew Cooper wrote:
> On 09/02/16 04:15, Doug Goldstein wrote:
>> diff --git a/xen/include/asm-x86/vpmu.h b/xen/include/asm-x86/vpmu.h
>> index 67e73dc..4750a1f 100644
>> --- a/xen/include/asm-x86/vpmu.h
>> +++ b/xen/include/asm-x86/vpmu.h
>> @@ -89,10 +89,14 @@ static inline void vpmu_clear(struct vpmu_struct *vpmu)
>>   {
>>       vpmu->flags = 0;
>>   }
>> +#ifdef CONFIG_XENOPROF
>>   static inline bool_t vpmu_is_set(const struct vpmu_struct *vpmu, const u32 mask)
>>   {
>>       return !!(vpmu->flags & mask);
>>   }
>> +#else
>> +#define vpmu_is_set(x, y) (!!0)
> Why vpmu_is_set()?  this wasn't guarded in v1.

This looks wrong to me. vpmu_is_set() is almost exclusively used by VPMU 
code.

>
>> +#endif
>>   static inline bool_t vpmu_are_all_set(const struct vpmu_struct *vpmu,
>>                                         const u32 mask)
>>   {
>> @@ -121,8 +125,13 @@ static inline int vpmu_do_rdmsr(unsigned int msr, uint64_t *msr_content)
>>       return vpmu_do_msr(msr, msr_content, 0, 0);
>>   }
>>   
>> +#ifdef CONFIG_XENOPROF
>>   extern int acquire_pmu_ownership(int pmu_ownership);
>>   extern void release_pmu_ownership(int pmu_ownership);
>> +#else
>> +#define acquire_pmu_ownership(x) (1)
>> +#define release_pmu_ownership(x)

Hmm... acquire_pmu_ownship() vs acquire_pmu_ownership() and 
release_pmu_ownship() vs. release_pmu_ownership().

Since you are working with this code, can you also clean that up? It 
looks to me that at some point names got messed up.

-boris