Re: [PATCH v9 2/5] KVM: stats: Add fd-based API to read binary stats data

On Wed, Jun 16, 2021 at 12:12 PM Paolo Bonzini [off-list ref] wrote:

On 14/06/21 23:21, Jing Zhang wrote:

quoted

+     /* Copy kvm stats values */
+     copylen = header->header.data_offset + size_stats - pos;
+     copylen = min(copylen, remain);
+     if (copylen > 0) {
+             src = stats + pos - header->header.data_offset;
+             if (copy_to_user(dest, src, copylen))
+                     return -EFAULT;
+             remain -= copylen;
+             pos += copylen;
+             dest += copylen;
+     }

Hi Jing,

this code is causing usercopy warnings because the statistics are not
part of the vcpu slab's usercopy region.  You need to move struct
kvm_vcpu_stat next to struct kvm_vcpu_arch, and adjust the call to
kmem_cache_create_usercopy in kvm_init.

Can you post a new version of the series, and while you are at it
explain the rationale for binary stats in the commit message for this
patch?  This should include:

- the problem statement (e.g. frequency of the accesses)

- what are the benefits compared to debugfs

- why the schema is included in the file descriptor as well

You can probably find a lot or all of the information in my emails from
the last couple days, but you might also have other breadcrumbs from
Google's internal implementation of binary stats.

Thanks,

Paolo

Hi Paolo

Will fix the usercopy warnings, add more explanations and post another version.
And thanks for all the information in your emails.

Jing