On Wed, Jan 15, 2025 at 5:51 PM Keith Busch <kbusch@xxxxxxxxxx> wrote:
>
> On Tue, Jan 14, 2025 at 07:06:20PM -0800, Sean Christopherson wrote:
> > > > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> > > > index 2401606db2604..422b6b06de4fe 100644
> > > > --- a/arch/x86/kvm/mmu/mmu.c
> > > > +++ b/arch/x86/kvm/mmu/mmu.c
> > > > @@ -7415,6 +7415,8 @@ int kvm_mmu_post_init_vm(struct kvm *kvm)
> > > > {
> > > > if (nx_hugepage_mitigation_hard_disabled)
> > > > return 0;
> > > > + if (kvm->arch.nx_huge_page_recovery_thread)
> > > > + return 0;
> >
> > ...
> >
> > > > kvm->arch.nx_huge_page_last = get_jiffies_64();
> > > > kvm->arch.nx_huge_page_recovery_thread = vhost_task_create(
> > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > > > index c79a8cc57ba42..263363c46626b 100644
> > > > --- a/arch/x86/kvm/x86.c
> > > > +++ b/arch/x86/kvm/x86.c
> > > > @@ -11463,6 +11463,10 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
> > > > struct kvm_run *kvm_run = vcpu->run;
> > > > int r;
> > > > + r = kvm_mmu_post_init_vm(vcpu->kvm);
> > > > + if (r)
> > > > + return r;
> >
> > The only lock held at this point is vcpu->mutex, the obvious choices for guarding
> > the per-VM task creation are kvm->lock or kvm->mmu_lock, but we definitely don't
> > want to blindly take either lock in KVM_RUN.
>
> Thanks for the feedback. Would this otherwise be okay if I use a
> different mechanism to ensure the vhost task creation happens only once
> per kvm instance? Or are you suggesting the task creation needs to be
> somewhere other than KVM_RUN?
You can implement something like pthread_once():
#define ONCE_NOT_STARTED 0
#define ONCE_RUNNING 1
#define ONCE_COMPLETED 2
struct once {
atomic_t state;
struct mutex lock;
}
static inline void __once_init(struct once *once,
const char *name, struct lock_class_key *key)
{
atomic_set(&once->state, ONCE_NOT_STARTED);
__mutex_init(&once->lock, name, key);
}
#define once_init(once) \
do { \
static struct lock_class_key __key; \
\
__once_init((once), #once, &__key); \
} while (0)
static inline void call_once(struct once *once, void (*cb)(struct once *))
{
/* Pairs with atomic_set_release() below. */
if (atomic_read_acquire(&once->state) == ONCE_COMPLETED)
return;
guard(mutex)(&once->lock);
WARN_ON(atomic_read(&once->state) == ONCE_RUNNING);
if (atomic_read(&once->state) != ONCE_NOT_STARTED)
return;
atomic_set(&once->state, ONCE_RUNNING);
cb(once);
atomic_set_release(&once->state, ONCE_COMPLETED);
}
Where to put it I don't know. It doesn't belong in
include/linux/once.h. I'm okay with arch/x86/kvm/call_once.h and just
pull it with #include "call_once.h".
Paolo
