From: Sean Christopherson <seanjc@google.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
zhanghao <zhanghao1@kylinos.cn>,
Wanpeng Li <kernellwp@gmail.com>
Subject: Re: [PATCH] KVM: Drop kvm_vcpu.ready to squash race where "ready" can get stuck "true"
Date: Tue, 14 Apr 2026 15:38:58 -0700 [thread overview]
Message-ID: <ad7CAgFO53yNrT_L@google.com> (raw)
In-Reply-To: <cb458ee1-e0c7-43e2-83a0-9effd9a723af@redhat.com>
On Tue, Apr 14, 2026, Paolo Bonzini wrote:
> On 4/9/26 23:33, Sean Christopherson wrote:
> > +static inline bool kvm_vcpu_is_runnable_and_scheduled_out(struct kvm_vcpu *vcpu)
> > +{
> > + return READ_ONCE(vcpu->preempted) ||
> > + (READ_ONCE(vcpu->scheduled_out) &&
> > + READ_ONCE(vcpu->wants_to_run) &&
>
> wants_to_run doesn't seem important here, because blocking will never be set
> outside KVM_RUN (unlike scheduled_out which can be set within any
> vcpu_load/vcpu_put pair, if you're unlucky enough).
Oh, good point.
> > + READ_ONCE(vcpu->stat.generic.blocking) &&
> > + !kvm_vcpu_is_blocking(vcpu));
>
> If you get here you have done the finish_rcuwait() in kvm_vcpu_block(),
> meaning that you've been already scheduled in, haven't you?
Gah, yes. I didn't realize finish_rcuwait() is what actually completes the
wakeup from KVM's perspective.
> So, you would need something like this:
>
> static inline bool kvm_vcpu_is_runnable_and_scheduled_out(struct kvm_vcpu *vcpu)
> {
> if (READ_ONCE(vcpu->preempted))
> return true;
>
> if (!READ_ONCE(vcpu->scheduled_out))
> return false;
> if (!READ_ONCE(vcpu->stat.generic.blocking))
Hmm, I think this could actually be:
if (!kvm_vcpu_is_blocking(vcpu))
return false;
Because my use of vcpu->stat.generic.blocking was purely due to missing that
finish_rcuwait() is effectively what clears "blocking". That would narrow the
window for false positives a little, e.g. would at least wait until after the
kvm_arch_vcpu_blocking() call to treat the vCPU as blocking.
> return false;
> return rcuwait_was_woken(kvm_arch_vcpu_get_wait(vcpu));
> }
>
> // in rcuwait.h
> static inline bool rcuwait_was_woken(struct rcuwait *w)
> {
> guard(rcu)();
> struct task_struct *t = rcu_access_pointer(w->task);
> return t && !task_is_runnable(t);
Ah, and I missed the task_is_runnable() check guarding vcpu->ready. I suspect I
assumed kvm_vcpu_on_spin() would do that check.
prev parent reply other threads:[~2026-04-14 22:39 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-09 21:33 [PATCH] KVM: Drop kvm_vcpu.ready to squash race where "ready" can get stuck "true" Sean Christopherson
2026-04-14 9:09 ` zhanghao
2026-04-14 13:44 ` Sean Christopherson
2026-04-14 16:06 ` Paolo Bonzini
2026-04-14 22:38 ` Sean Christopherson [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ad7CAgFO53yNrT_L@google.com \
--to=seanjc@google.com \
--cc=kernellwp@gmail.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=zhanghao1@kylinos.cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.