Re: [PATCH] KVM: x86: Make the debugfs per VM optional

[Sean Christopherson <seanjc@google.com>]

On Thu, Nov 07, 2024, Paolo Bonzini wrote:
> On Fri, Nov 1, 2024 at 6:15 PM Sean Christopherson <seanjc@google.com> wrote:
> > I'm not opposed to letting userspace say "no debugfs for me", but I don't know
> > that a module param is the right way to go.  It's obviously quite easy to
> > implement and maintain (in code), but I'm mildly concerned that it'll have limited
> > usefulness and/or lead to bad user experiences, e.g. because people turn off debugfs
> > for startup latency without entirely realizing what they're sacrificing.
> 
> What are they sacrificing? :)

For all intents and purposes, the ability to get an per-VM and per-vCPU information
from an arbitrary shell.

> The per-VM statistics information is also accessible without debugfs, even
> though kvm_stat does not support it.

I assume you're referring to KVM_GET_STATS_FD?  That's not easy to get at from
the shell.

If a host is running a single VM, then the per-VM directories aren't needed.  But
I would be very, very surprised if there's a legitimate use case for running a
single VM, with debugfs, that cares deeply about the boot latency of that one VM.

FWIW, I would be wholeheartedly in favor of providing tooling to get at stats
via KVM_GET_STATS_FD, e.g. given a VM's PID.  But then I think it would make sense
to have CONFIG_KVM_DEBUGFS, not a module param.

> However I'd make the module parameter read-only, so you don't have
> half-and-half setups. And maybe even in this mode we should create the
> directory anyway to hold the vcpu%d/pid files, which are not
> accessible in other ways.
> 
> > One potentially terrible idea would be to setup debugfs asynchronously, so that
> > the VM is runnable asap, but userspace still gets full debugfs information.  The
> > two big wrinkles would be the vCPU debugfs creation and kvm_uevent_notify_change()
> > (or at least the STATS_PATH event) would both need to be asynchronous as well.
> 
> STATS_PATH is easy because you can create the toplevel directory
> synchronously; same for vCPUs. I'd be willing to at least see what a
> patch looks like.

Ah, creating the directories synchrously would definitely simplify things.