All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Yan Zhao <yan.y.zhao@intel.com>
Cc: pbonzini@redhat.com, reinette.chatre@intel.com,
	rick.p.edgecombe@intel.com,  linux-kernel@vger.kernel.org,
	kvm@vger.kernel.org
Subject: Re: [PATCH v2 3/3] KVM: selftests: Test prefault memory during concurrent memslot removal
Date: Mon, 15 Sep 2025 17:14:50 -0700	[thread overview]
Message-ID: <aMir-qs5zwmoXU6A@google.com> (raw)
In-Reply-To: <aMfMk/x5XJ1bfvzv@yzhao56-desk.sh.intel.com>

On Mon, Sep 15, 2025, Yan Zhao wrote:
> On Mon, Sep 08, 2025 at 04:47:23PM -0700, Sean Christopherson wrote:
> > On Fri, Aug 22, 2025, Yan Zhao wrote:
> > +		if (!slot_recreated) {
> > +			WRITE_ONCE(data.recreate_slot, true);
> > +			pthread_join(slot_worker, NULL);
> > +			slot_recreated = true;
> > +			continue;
> If delete_slot_worker() invokes vm_mem_region_delete() slowly enough due to
> scheduling delays, the return value from __vcpu_ioctl() could be 0 with
> range.size being 0 at this point.
> 
> What about checking range.size before continuing?
> 
> @@ -120,7 +126,8 @@ static void pre_fault_memory(struct kvm_vcpu *vcpu, u64 base_gpa, u64 offset,
>                         WRITE_ONCE(data.recreate_slot, true);
>                         pthread_join(slot_worker, NULL);
>                         slot_recreated = true;
> -                       continue;
> +                       if (range.size)
> +                               continue;
>                 }
> 
> 
> Otherwise, the next __vcpu_ioctl() would return -1 with errno == EINVAL, which
> will break the assertion below.

Drat, I missed that kvm_vcpu_pre_fault_memory() returns -EINVAL on a size of '0'
(see the wrong comment snippet "Either prefaulting already succeeded, in which
case retrying should also succeed, or retry is needed to get a stable result").

I'll circle back to this tomorrow.  IIRC, there was a reason I didn't want to
check range.size in that path, but for the life of me I can't remember why :-/

  reply	other threads:[~2025-09-16  0:14 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-22  7:03 [PATCH v2 0/3] KVM: Fix deadlock for invalid memslots Yan Zhao
2025-08-22  7:03 ` [PATCH v2 1/3] KVM: x86/mmu: Return -EAGAIN if userspace deletes/moves memslot during prefault Yan Zhao
2025-09-09  2:46   ` Binbin Wu
2025-08-22  7:05 ` [PATCH v2 2/3] KVM: TDX: Do not retry locally when the retry is caused by invalid memslot Yan Zhao
2025-09-09  3:29   ` Binbin Wu
2025-09-09 14:18     ` Sean Christopherson
2025-09-10  2:02       ` Binbin Wu
2025-08-22  7:05 ` [PATCH v2 3/3] KVM: selftests: Test prefault memory during concurrent memslot removal Yan Zhao
2025-09-08 23:47   ` Sean Christopherson
2025-09-15  8:21     ` Yan Zhao
2025-09-16  0:14       ` Sean Christopherson [this message]
2025-09-24 17:09         ` Sean Christopherson
2025-09-16  0:25 ` [PATCH v2 0/3] KVM: Fix deadlock for invalid memslots Sean Christopherson

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=aMir-qs5zwmoXU6A@google.com \
    --to=seanjc@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=reinette.chatre@intel.com \
    --cc=rick.p.edgecombe@intel.com \
    --cc=yan.y.zhao@intel.com \
    /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.