All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: "shaikh.kamal" <shaikhkamal2012@gmail.com>,
	"H. Peter Anvin" <hpa@zytor.com>,  Paul Durrant <paul@xen.org>,
	David Woodhouse <dwmw@amazon.co.uk>,
	kvm@vger.kernel.org,  linux-kernel@vger.kernel.org,
	linux-rt-devel@lists.linux.dev,  pbonzini@redhat.com,
	skhan@linuxfoundation.org, me@brighamcampbell.com,
	 syzbot+919877893c9d28162dc2@syzkaller.appspotmail.com
Subject: Re: [PATCH v2 1/1] KVM: x86/xen: Use trylock for fast path event channel delivery
Date: Thu, 2 Apr 2026 15:40:21 -0700	[thread overview]
Message-ID: <ac7wVQDWiXvurkZo@google.com> (raw)
In-Reply-To: <20260402063612.VVXEy0qn@linutronix.de>

On Thu, Apr 02, 2026, Sebastian Andrzej Siewior wrote:
> On 2026-04-02 07:01:02 [+0530], shaikh.kamal wrote:
> …
> > The function uses read_lock_irqsave() to access two gpc structures:
> > shinfo_cache and vcpu_info_cache. On PREEMPT_RT, these rwlocks are
> > rt_mutex-based and cannot be acquired from hard IRQ context.
> > 
> > Use read_trylock() instead for both gpc lock acquisitions. If either
> > lock is contended, return -EWOULDBLOCK to trigger the existing slow
> > path: xen_timer_callback() sets vcpu->arch.xen.timer_pending, kicks
> > the vCPU with KVM_REQ_UNBLOCK, and the event gets injected from
> > process context via kvm_xen_inject_timer_irqs().
> > 
> > This approach works on all kernels (RT and non-RT) and preserves the
> > "fast path" semantics: acquire the lock only if immediately available,
> > otherwise bail out rather than blocking.
> 
> No. This split into local_irq_save() + trylock is something you must not
> do. The fact that it does not lead to any warnings does not mean it is
> good.
> One problem is that your trylock will record the current task on the CPU
> as the owner of this lock which can lead to odd lock chains if observed
> by other tasks while trying to PI.

Is that a problem with local_irq_save() specifically, or is it a broader problem
with doing read_trylock() inside a raw spinlock?  (Or using read_trylock() in
the sched_out path in particular?)

I ask because I _think_ David's suggestion was to drop the irq_save stuff
entirely, because if KVM only ever does trylock, there's no risk of deadlocking
due to waiting on the lock in atomic context.

> So no.
> 
> If this is just to shut up syskaller I would suggest to let xen depend
> on !PREEMPT_RT until someone figures out what to do.

Heh, I was considering proposing exactly that, but it doesn't actually change
anything in practice, because no one actually use KVM XEN support with PREEMPT_RT.
Making the two mutually exclusive would completely prevent the badness, but it
wouldn't fix the more annoying (for me at least) problem, which is that
check_wait_context() fires with CONFIG_PROVE_LOCKING=y irrespective of PREEMPT_RT.
I.e. making CONFIG_KVM_XEN depend on !PREEMPT_RT won't eliminate what are already
false positives.

More importantly, there's a desire to use the same KVM construct in other code
that runs inside the shced_out() path and thus attempts to take a non-raw rwlock
inside a raw spinlock[*].  And that code isn't mutually exclusive with PREEMPT_RT.
So while I'd be happy to punt on XEN, the underlying problem needs to be solved :-/.

https://lore.kernel.org/all/1d6712ed413ea66ef376d1410811997c3b416e99.camel@infradead.org

  reply	other threads:[~2026-04-02 22:40 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-29 13:15 [PATCH] KVM: x86/xen: Fix sleeping lock in hard IRQ context on PREEMPT_RT shaikh.kamal
2026-03-30 14:18 ` Steven Rostedt
2026-03-30 14:51   ` Woodhouse, David
2026-04-01 15:40     ` Sean Christopherson
2026-04-02  1:30       ` [PATCH v2 0/1] KVM: x86/xen: Fix PREEMPT_RT sleeping lock bug shaikh.kamal
2026-04-02  1:31       ` [PATCH v2 1/1] KVM: x86/xen: Use trylock for fast path event channel delivery shaikh.kamal
2026-04-02  6:36         ` Sebastian Andrzej Siewior
2026-04-02 22:40           ` Sean Christopherson [this message]
2026-04-02  6:42       ` [PATCH] KVM: x86/xen: Fix sleeping lock in hard IRQ context on PREEMPT_RT Sebastian Andrzej Siewior
2026-04-02 22:23         ` Sean Christopherson
2026-04-29 22:25       ` [PATCH v2 0/1] mm/mmu_notifier: Add async OOM cleanup via call_srcu() shaikh.kamal
2026-04-29 22:25       ` [PATCH v2 1/1] " shaikh.kamal
2026-05-03  3:26         ` kernel test robot
2026-05-03  3:26         ` kernel test robot

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=ac7wVQDWiXvurkZo@google.com \
    --to=seanjc@google.com \
    --cc=bigeasy@linutronix.de \
    --cc=dwmw@amazon.co.uk \
    --cc=hpa@zytor.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rt-devel@lists.linux.dev \
    --cc=me@brighamcampbell.com \
    --cc=paul@xen.org \
    --cc=pbonzini@redhat.com \
    --cc=shaikhkamal2012@gmail.com \
    --cc=skhan@linuxfoundation.org \
    --cc=syzbot+919877893c9d28162dc2@syzkaller.appspotmail.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.