Subject: Re: [PATCH V2 4/5] kvm/stats: Add provisioning for 64-bit vcpu statistics

From: Christian Borntraeger <[email-masked]>
Date: Mon, 18 Jul 2016 09:17:58 +0200

On 07/15/2016 09:52 AM, Suraj Jitindar Singh wrote:
> 
> 
> On 14/07/16 19:42, Paolo Bonzini wrote:
>>
>> On 13/07/2016 20:00, Christian Borntraeger wrote:
>>>>>>> I thought u64 still existed on 32-bit architectures. unsigned long
>>>>>>> would be fine but with the caveat that certain stats would overflow on
>>>>>>> 32-bit architectures.
>>>>> Yes, but not all 32-bit architectures can do atomic read-modify-write
>>>>> (e.g. add) operations on 64-bit values.
>>> So what about only doing it for the VCPU events? Those should be only
>>> modified by one CPU. We would have some odd values on 32bit overflow, but
>>> this will be certainly better than just start with 0
>> If that's good enough for PPC, that's fine.
>>
>> Paolo
> 
> I'm don't feel great about having vcpu_stats as u64 and vm_stats still as u32
> it's just a bit inconsistent.
> 
> That being said, it's only the vcpu_stats which I require to be u64 at this
> stage so it's possible to just upgrade those.

Yes, its not nice, but we probably want to avoid the overhead of atomics.
What about using u64 for vcpu_stats and unsigned long for vm_stats. This will be
correct for anyone and on 64bit systems we get 64 bits for everything?