From: Sean Christopherson <seanjc@google.com>
To: David Woodhouse <dwmw2@infradead.org>
Cc: pbonzini@redhat.com, tglx@kernel.org, mingo@redhat.com,
bp@alien8.de, dave.hansen@linux.intel.com, x86@kernel.org,
hpa@zytor.com, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, sashiko-reviews@lists.linux.dev,
dwmw@amazon.co.uk
Subject: Re: [PATCH v2 15/20] KVM: x86/xen: Convert kvm_xen_set_evtchn_fast() to gpc's CLASS() APIs
Date: Mon, 8 Jun 2026 17:53:56 -0700 [thread overview]
Message-ID: <aidkJJesvO9vE77j@google.com> (raw)
In-Reply-To: <c1f0dee379e282ffb35296da4b666d93025f1c95.camel@infradead.org>
On Tue, Jun 02, 2026, David Woodhouse wrote:
> On Fri, 29 May 2026 12:11:37 -0700, Sean Christopherson wrote:
> > On Fri, May 29, 2026, sashiko-bot@kernel.org wrote:
> > > [Severity: High]
> > > Does this unintentionally hold the gpc lock across the IPI kick?
> >
> > No, it intentionally holds the gpc lock across _sending_ the IPI kick.
> >
> > > [Severity: High]
> > > Does this now hold the VM-wide shinfo_cache lock while calling
> > > __kvm_xen_set_evtchn_fast() and kicking the vCPU?
> > >
> > > Since shinfo_map is a function-scoped CLASS() variable, its destructor
> > > won't release the lock until after __kvm_xen_set_evtchn_fast() returns.
> > > This creates a nested locking dependency and holds locks over expensive
> > > cross-vCPU operations, potentially serializing event channel deliveries
> > > across the entire VM on the fast path.
> >
> > __kvm_vcpu_kick() is neither expensive nor cross-vCPU. In the wait=false case,
> > which is the behavior of kvm_vcpu_kick(), it sends IPIs via smp_send_reschedule(),
> > i.e. it's more or less just __apic_send_IPI(cpu, RESCHEDULE_VECTOR), which is a
> > single WRMSR on modern harware.
>
> Hm, if we really are going to use a raw rwlock then I suppose we should
> take care not to spend any extra cycles with the lock held unless we
> actually *need* to?
>
> In which case I wonder if we really need the scoped CLASS() thing (that
> encourages people to run to the end of the function under the lock), or
> if the helpers you add for that are sufficient on their own?
Eh, I would argue that "the code is easier to read" is sufficient justification
so long as holding the lock for a few tens of extra cycles doesn't cause problems
in practice.
next prev parent reply other threads:[~2026-06-09 0:53 UTC|newest]
Thread overview: 57+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-29 16:50 [PATCH v2 00/20] KVM: x86/xen: Fix Xen/GP/PREEMPT_RT issues with rwlock_t Sean Christopherson
2026-05-29 16:50 ` [PATCH v2 01/20] locking/rt: Use raw_spin_lock_irqsave() in __rwbase_read_unlock() Sean Christopherson
2026-05-29 19:32 ` Peter Zijlstra
2026-05-29 19:34 ` Peter Zijlstra
2026-05-29 20:05 ` Sean Christopherson
2026-05-29 20:13 ` Peter Zijlstra
2026-05-29 20:38 ` Peter Zijlstra
2026-05-30 0:54 ` Sean Christopherson
2026-05-30 10:26 ` Paolo Bonzini
2026-05-30 12:47 ` David Woodhouse
2026-05-30 14:40 ` Paolo Bonzini
2026-06-01 10:52 ` David Woodhouse
2026-06-01 13:01 ` David Woodhouse
2026-06-01 13:40 ` Sebastian Andrzej Siewior
2026-06-01 13:53 ` David Woodhouse
2026-06-01 14:47 ` Sebastian Andrzej Siewior
2026-06-01 15:11 ` David Woodhouse
2026-06-01 9:40 ` Peter Zijlstra
2026-06-01 10:04 ` David Woodhouse
2026-05-30 13:02 ` Paolo Bonzini
2026-06-01 8:40 ` Peter Zijlstra
2026-06-01 11:11 ` Sebastian Andrzej Siewior
2026-06-01 11:40 ` Peter Zijlstra
2026-06-01 19:13 ` Paolo Bonzini
2026-06-02 7:34 ` Sebastian Andrzej Siewior
2026-06-04 23:58 ` David Woodhouse
2026-05-29 16:50 ` [PATCH v2 02/20] KVM: x86/xen: Use read_trylock() for GPC locks in hardirq/atomic paths Sean Christopherson
2026-05-29 17:20 ` sashiko-bot
2026-05-29 23:28 ` Hillf Danton
2026-05-29 16:50 ` [PATCH v2 03/20] KVM: x86/xen: Remove unnecessary irqsave from GPC lock usage in xen.c Sean Christopherson
2026-05-29 17:36 ` sashiko-bot
2026-05-29 16:50 ` [PATCH v2 04/20] KVM: x86: Remove unnecessary irqsave from kvm_setup_guest_pvclock() Sean Christopherson
2026-05-29 16:50 ` [PATCH v2 05/20] KVM: Remove unnecessary IRQ disabling from GPC lock in pfncache.c Sean Christopherson
2026-05-29 16:51 ` [PATCH v2 06/20] KVM: x86/xen: Use guard() to grab kvm->srcu around gpc critical sections Sean Christopherson
2026-05-29 16:51 ` [PATCH v2 07/20] KVM: x86/xen: Extract delivery of event to vCPU into a separate helper Sean Christopherson
2026-05-29 17:47 ` sashiko-bot
2026-05-29 16:51 ` [PATCH v2 08/20] KVM: x86/xen: Explicitly tag "shared info" page as never being dirty tracked Sean Christopherson
2026-05-29 16:51 ` [PATCH v2 09/20] KVM: x86/xen: Don't dirty track "vCPU info" page Sean Christopherson
2026-05-29 16:51 ` [PATCH v2 10/20] KVM: Move {g,p}fn <=> {g,h}pa conversion helpers to kvm_types.h Sean Christopherson
2026-05-29 16:51 ` [PATCH v2 11/20] KVM: Add CLASS() constructs to automagically handle lock+check of gpc Sean Christopherson
2026-05-29 17:59 ` sashiko-bot
2026-05-29 16:51 ` [PATCH v2 12/20] KVM: x86/xen: Convert kvm_xen_shared_info_init() to gpc's CLASS() APIs Sean Christopherson
2026-05-29 16:51 ` [PATCH v2 13/20] KVM: x86/xen: Don't bother waiting on gpc->lock in SCHEDOP_poll Sean Christopherson
2026-05-29 16:51 ` [PATCH v2 14/20] KVM: x86/xen: Convert wait_pending_event() to gpc's CLASS() APIs Sean Christopherson
2026-05-29 16:51 ` [PATCH v2 15/20] KVM: x86/xen: Convert kvm_xen_set_evtchn_fast() " Sean Christopherson
2026-05-29 19:01 ` sashiko-bot
2026-05-29 19:11 ` Sean Christopherson
2026-06-02 12:37 ` David Woodhouse
2026-06-09 0:53 ` Sean Christopherson [this message]
2026-05-29 16:51 ` [PATCH v2 16/20] KVM: x86/xen: Convert xen_get_guest_pvclock() " Sean Christopherson
2026-05-29 16:51 ` [PATCH v2 17/20] KVM: x86/xen: Drop local "kick_vcpu" from __kvm_xen_set_evtchn_fast() Sean Christopherson
2026-05-29 16:51 ` [PATCH v2 18/20] KVM: x86/xen: Convert event injection to gpc's CLASS() APIs Sean Christopherson
2026-05-29 16:51 ` [PATCH v2 19/20] KVM: Add "extended" gpc CLASS() APIs for sometimes-atomic cases Sean Christopherson
2026-05-29 16:51 ` [PATCH v2 20/20] KVM: x86: Use gfn_to_pfn_cache for record_steal_time Sean Christopherson
2026-05-30 6:19 ` sashiko-bot
2026-06-02 12:29 ` David Woodhouse
2026-06-09 0:45 ` 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=aidkJJesvO9vE77j@google.com \
--to=seanjc@google.com \
--cc=bp@alien8.de \
--cc=dave.hansen@linux.intel.com \
--cc=dwmw2@infradead.org \
--cc=dwmw@amazon.co.uk \
--cc=hpa@zytor.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=pbonzini@redhat.com \
--cc=sashiko-reviews@lists.linux.dev \
--cc=tglx@kernel.org \
--cc=x86@kernel.org \
/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.