On Wed, Jun 16, 2021 at 12:12 PM Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote:
>
> On 14/06/21 23:21, Jing Zhang wrote:
> > + /* 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
