Re: [PATCH 1/4] KVM: stats: Make sure no missing or mismatched binary stats definition

Hi Fuad,

On Mon, Jun 14, 2021 at 4:50 AM Fuad Tabba [off-list ref] wrote:

Hi Jing,

On Mon, Jun 14, 2021 at 3:53 AM Jing Zhang [off-list ref] wrote:

quoted

Add static check to make sure the number of stats descriptors equals
the number of stats defined in vm/vcpu stats structures.
Add offset field in stats descriptor to let us define stats
descriptors freely, don't have to be in the same order as
stats in vm/vcpu stats structures.
Also fix some missing/mismatched stats from previous patch.

Signed-off-by: Jing Zhang <redacted>

I tested this for arm64, and it does assert if there's a mismatch. I
couldn't find any missing statistics under any of the architectures
either.

quoted

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 5e77f32abef5..692af9177c9f 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h

@@ -1296,119 +1296,152 @@ struct _kvm_stats_desc {
        { n, offsetof(struct kvm_vcpu, stat.generic.x),                        \
          KVM_STAT_VCPU, ## __VA_ARGS__ }

-#define STATS_DESC(stat, type, unit, base, exp)                               \
+#define STATS_DESC_COMMON(type, unit, base, exp)                              \
+       .flags = type | unit | base |                                          \
+           BUILD_BUG_ON_ZERO(type & ~KVM_STATS_TYPE_MASK) |                   \
+           BUILD_BUG_ON_ZERO(unit & ~KVM_STATS_UNIT_MASK) |                   \
+           BUILD_BUG_ON_ZERO(base & ~KVM_STATS_BASE_MASK),                    \
+       .exponent = exp,                                                       \
+       .size = 1
+

nit: you seem to be mixing tabs and spaces here

quoted

+#define VM_GENERIC_STATS_DESC(stat, type, unit, base, exp)                    \
        {                                                                      \
                {                                                              \
-                       .flags = type | unit | base |                          \
-                           BUILD_BUG_ON_ZERO(type & ~KVM_STATS_TYPE_MASK) |   \
-                           BUILD_BUG_ON_ZERO(unit & ~KVM_STATS_UNIT_MASK) |   \
-                           BUILD_BUG_ON_ZERO(base & ~KVM_STATS_BASE_MASK),    \
-                       .exponent = exp,                                       \
-                       .size = 1                                              \
+                       STATS_DESC_COMMON(type, unit, base, exp),              \
+                       .offset = offsetof(struct kvm_vm_stat, generic.stat)   \
                },                                                             \
-               .name = stat,                                                  \
+               .name = #stat,                                                 \
        }

nit: also here, mixing of tabs and spaces

Tested-by: Fuad Tabba <redacted> #arm64
Reviewed-by: Fuad Tabba <redacted>

Thanks,
/fuad

Thanks for the review and testing!

Jing