Re: [PATCH v5 3/4] KVM: s390: Change the fi->lock to a raw_spinlock for RT case

[Douglas Freimuth]

On 5/7/26 10:45 AM, Heiko Carstens wrote:
> Adding Peter :)
>
> On Thu, May 07, 2026 at 09:17:00AM -0400, Matthew Rosato wrote:
> > On 5/7/26 5:56 AM, Heiko Carstens wrote:
> > > On Wed, May 06, 2026 at 10:50:52AM -0400, Douglas Freimuth wrote:
> > > > On 5/6/26 12:57 AM, Heiko Carstens wrote:
> > > > > On Tue, May 05, 2026 at 07:37:27PM +0200, Douglas Freimuth wrote:
> > > > > > s390 needs to maintain support for an RT kernel. This requires the
> > > > > > floating interrupt lock, fi->lock to be changed to a raw spin lock
> > > > > > since the fi->lock maybe called with interrupts disabled in __inject_io.
> > > > > >
> > > > > > Signed-off-by: Douglas Freimuth <freimuth@xxxxxxxxxxxxx>
> > > > > > ---
> > > > > >    arch/s390/include/asm/kvm_host.h |  2 +-
> > > > > >    arch/s390/kvm/intercept.c        |  4 +-
> > > > > >    arch/s390/kvm/interrupt.c        | 68 ++++++++++++++++----------------
> > > > > >    arch/s390/kvm/kvm-s390.c         |  2 +-
> > > > > >    4 files changed, 38 insertions(+), 38 deletions(-)
> > > > >
> > > > > s390 does not support RT, but I guess you are referring to a lockdep splat
> > > > > which you would see without doing this change, similar like we have seen at
> > > > > other places.
> > > > >
> > > > > Can you include the relevant parts of the splat for reference, please?
>
> ...
>
> > AFAIU it is only problematic if we (s390) should ever want to support RT
> > in the future.
>
> I don't see that coming, but nobody knows what happens in future.
>
> ...
>
> > My original thinking was 'well, it won't hurt to use the raw spinlocks
> > in the new code' so I set Doug down this road with my review comments --
> > I did not consider that there would be a need for additional fallout
> > like this patch, which means increased chance of regressions (see below)
> > to accomodate a feature that we don't support today.
> >
> > If you are saying it's OK to simply not care about RT for s390 now, then
> > AFAICT it should be fine to just use s/raw_spin_)lock/spin_lock/ for
> > this whole series, drop this patch and then ignore the subsequent
> > Sashiko complaints about RT.
> >
> > What do you think?
>
> So... after having given this a second thought: we do not have
> PROVE_RAW_LOCK_NESTING enabled in our debug_defconfig (either we missed it,
> or somebody (cough) thought it is not relevant for s390). That said, I
> believe we should enable it, fix all fallout and also make sure that new
> code does not generate any lockdep splats with PROVE_RAW_LOCK_NESTING
> enabled.
>
> Rationale: even though it is not relevant for s390, we also change common
> code; and by ignoring PROVE_RAW_LOCK_NESTING we might cause problems for
> other architectures by introducing incorrect nesting of locks in common
> code. So yes, your thinking is correct.

Heiko, to be complete, I went through the exercise of enabling 
PROVE_RAW_LOCK_NESTING. I created a small hack to generate a 
__deliver_machine_check to trap the nested locking issue. The requested 
splat is below. Here the floating interrupt lock is a raw_spin_lock and 
the nested call to local interrupt lock is a spin_lock thus the nesting 
issue. No other nesting issues were found.

Now we need to arrive at, do we keep the raw_spin_locks to cover the 
possibility of future RT support or common code? In that case I also 
make the li->lock a raw_spin_lock. OR should I drop this raw_spin_lock 
patch and back out any other raw_spin_locks since we dont currently 
support RT on s390? And end either choice by testing again with 
PROVE_RAW_LOCK_NESTING.

[  187.278926] =============================
[  187.278927] [ BUG: Invalid wait context ]
[  187.278930] 7.1.0-rc1-gb8e991a47d4c-dirty #6 Not tainted
[  187.278932] -----------------------------
[  187.278933] CPU 0/KVM/4263 is trying to lock:
[  187.278935] 000000c7448982a0 
(&vcpu->arch.local_int.lock){+.+.}-{3:3}, at: 
__deliver_machine_check+0x44/0x1a0 [kvm]
[  187.278976] other info that might help us debug this:
[  187.278978] context-{5:5}
[  187.278979] 3 locks held by CPU 0/KVM/4263:
[  187.278981]  #0: 000000c7448980b8 (&vcpu->mutex){+.+.}-{4:4}, at: 
kvm_vcpu_ioctl+0xb8/0x9b0 [kvm]
[  187.279001]  #1: 000000c73a75b108 (&kvm->srcu){.+.+}-{0:0}, at: 
__vcpu_run+0x46/0x4f0 [kvm]
[  187.279024]  #2: 000000c73a758dd0 
(&kvm->arch.float_int.lock){+.+.}-{2:2}, at: 
__deliver_machine_check+0x3a/0x1a0 [kvm]
[  187.279046] stack backtrace:
[  187.279048] CPU: 10 UID: 107 PID: 4263 Comm: CPU 0/KVM Not tainted 
7.1.0-rc1-gb8e991a47d4c-dirty #6 PREEMPT
[  187.279050] Hardware name: IBM 9175 ME1 701 (LPAR)
[  187.279051] Call Trace:
[  187.279051]  [<000001cbdd2e7eea>] dump_stack_lvl+0xa2/0xe8
[  187.279054]  [<000001cbdd3ecd98>] __lock_acquire+0xe18/0x15c0
[  187.279057]  [<000001cbdd3ed62c>] lock_acquire.part.0+0xec/0x260
[  187.279059]  [<000001cbdd3ed84c>] lock_acquire+0xac/0x200
[  187.279061]  [<000001cbde401528>] _raw_spin_lock+0x58/0xb0
[  187.279063]  [<000001cb5dc5e734>] __deliver_machine_check+0x44/0x1a0 
[kvm]
[  187.279082]  [<000001cb5dc6057e>] 
kvm_s390_deliver_pending_interrupts+0x7e/0x990 [kvm]
[  187.279099]  [<000001cb5dc49934>] vcpu_pre_run+0x74/0x2d0 [kvm]
[  187.279117]  [<000001cb5dc558e8>] __vcpu_run+0xa8/0x4f0 [kvm]
[  187.279134]  [<000001cb5dc56400>] kvm_arch_vcpu_ioctl_run+0x140/0x320 
[kvm]
[  187.279152]  [<000001cb5dc35cc2>] kvm_vcpu_ioctl+0x142/0x9b0 [kvm]
[  187.279167]  [<000001cbdd7d0bda>] __s390x_sys_ioctl+0xea/0x120
[  187.279171]  [<000001cbde3ef868>] __do_syscall+0x168/0x750
[  187.279173]  [<000001cbde402d1a>] system_call+0x72/0x90
[  187.279175] INFO: lockdep is turned off.


> Peter, I just added you to cc, so you can correct me if I'm entirely wrong.