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

[Oliver Upton <oliver.upton@linux.dev>]

On Fri, Nov 01, 2024 at 10:15:00AM -0700, Sean Christopherson wrote:
> On Wed, Oct 23, 2024, Bernhard Kauer wrote:
> > Creating a debugfs directory for each virtual machine is a suprisingly
> > costly operation as one has to synchronize multiple cores. However, short
> > living VMs seldom benefit from it.
> > 
> > Since there are valid use-cases we make this feature optional via a
> > module parameter. Disabling it saves 150us in the hello microbenchmark.
> > 
> > Signed-off-by: Bernhard Kauer <bk@alpico.io>
> > ---
> >  virt/kvm/kvm_main.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> > index a48861363649..760e39cf86a8 100644
> > --- a/virt/kvm/kvm_main.c
> > +++ b/virt/kvm/kvm_main.c
> > @@ -94,6 +94,9 @@ unsigned int halt_poll_ns_shrink = 2;
> >  module_param(halt_poll_ns_shrink, uint, 0644);
> >  EXPORT_SYMBOL_GPL(halt_poll_ns_shrink);
> >  
> > +bool debugfs_per_vm = true;
> > +module_param(debugfs_per_vm, bool, 0644);
> 
> 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.

I'd be open to a Kconfig option that disables only KVM debugfs, assuming
there are people out there who want that *and* still need the rest of
debugfs facilities.

Even assuming well-intentioned userspace, a defensive user might want to
hide KVM's debugfs surfaces in case it exposed customer data.

Otherwise !CONFIG_DEBUG_FS would get the job done.

> 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.

Sounds like a pile o' bugs waiting to happen in a rather gently tested
part of the KVM, so hopefully we don't need to consider that route :)

-- 
Thanks,
Oliver