Re: [PATCH] KVM: x86/xen: bail in IRQ context on PREEMPT_RT in kvm_xen_set_evtchn_fast()

[David Woodhouse <dwmw2@infradead.org>]

[-- Attachment #1: Type: text/plain, Size: 3218 bytes --]

On Thu, 2026-05-07 at 15:21 +0200, Sebastian Andrzej Siewior wrote:
> On 2026-05-07 14:00:49 [+0100], David Woodhouse wrote:
> > On Thu, 2026-05-07 at 09:12 +0200, Sebastian Andrzej Siewior wrote:
> > > 
> > > So the cited patch does not look bad. That read-trylock should be fine
> > > on RT (as in I don't see anything wrong with it). What did happen to it?
> > 
> > The read_trylock() may be fine... but does a read_unlock() try to
> > "re-enable" hardirqs?
> 
> Yes, I missed it while looking for it. This could become a _irqsave().
> 
> We do this already for spinlock_t/ rt_spin_lock()/ _unlock() because it
> is used during early boot where interrupts are disabled and locks are
> acquired. So it needs to preserve the state and since rwlock_t is not
> (yet) used during early boot it was not done/ observed.
> 
> The try-lock on the read lock just increments a counter in order to
> acquire the lock. This is it. The spinlock_t on the other hand records
> the current context as owner which can lead to a mess. Therefore a
> trylock on a spinlock_t from hardirq context is wrong but it should be
> doable for rwlock_t. I don't see anything wrong with it (except for this
> one thing can be corrected).

Thanks. Something like this?

From 7199173d543d25333418537bcf07d6b2fce18ff1 Mon Sep 17 00:00:00 2001
From: David Woodhouse <dwmw@amazon.co.uk>
Date: Thu, 7 May 2026 14:39:22 +0100
Subject: [PATCH] locking/rt: Use raw_spin_lock_irqsave() in
 __rwbase_read_unlock()

__rwbase_read_unlock() uses raw_spin_lock_irq()/raw_spin_unlock_irq()
which unconditionally disables and re-enables interrupts. When
read_unlock() is called from hardirq context (e.g. after a successful
read_trylock() in a timer callback), the raw_spin_unlock_irq()
incorrectly re-enables interrupts within the hardirq handler.

This causes lockdep warnings ('hardirqs_on_prepare' from hardirq
context) and can lead to IRQ state corruption.

Switch to raw_spin_lock_irqsave()/raw_spin_unlock_irqrestore() to
preserve the caller's IRQ state, matching the pattern already used
for rt_spin_lock/unlock.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 kernel/locking/rwbase_rt.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/locking/rwbase_rt.c b/kernel/locking/rwbase_rt.c
index 82e078c0665a..25744862d627 100644
--- a/kernel/locking/rwbase_rt.c
+++ b/kernel/locking/rwbase_rt.c
@@ -153,8 +153,9 @@ static void __sched __rwbase_read_unlock(struct rwbase_rt *rwb,
 	struct rt_mutex_base *rtm = &rwb->rtmutex;
 	struct task_struct *owner;
 	DEFINE_RT_WAKE_Q(wqh);
+	unsigned long flags;
 
-	raw_spin_lock_irq(&rtm->wait_lock);
+	raw_spin_lock_irqsave(&rtm->wait_lock, flags);
 	/*
 	 * Wake the writer, i.e. the rtmutex owner. It might release the
 	 * rtmutex concurrently in the fast path (due to a signal), but to
@@ -167,7 +168,7 @@ static void __sched __rwbase_read_unlock(struct rwbase_rt *rwb,
 
 	/* Pairs with the preempt_enable in rt_mutex_wake_up_q() */
 	preempt_disable();
-	raw_spin_unlock_irq(&rtm->wait_lock);
+	raw_spin_unlock_irqrestore(&rtm->wait_lock, flags);
 	rt_mutex_wake_up_q(&wqh);
 }
 
-- 
2.43.0



[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]