Re: [PATCH v6 3/3] rust: sync: Add SpinLockIrq

[Thomas Gleixner <tglx@linutronix.de>]

On Mon, Sep 16 2024 at 17:28, Lyude Paul wrote:
> A variant of SpinLock that is expected to be used in noirq contexts, and
> thus requires that the user provide an kernel::irq::IrqDisabled to prove
> they are in such a context upon lock acquisition. This is the rust
> equivalent of spin_lock_irqsave()/spin_lock_irqrestore().

This fundamentally does not work with CONFIG_PREEMPT_RT. See:

   https://www.kernel.org/doc/html/latest/locking/locktypes.html

for further information. TLDR:

On RT enabled kernels spin/rw_lock are substituted by sleeping locks. So
you _cannot_ disable interrupts before taking the lock on RT enabled
kernels. That will result in a 'might_sleep()' splat.

There is a reason why the kernel has two distinct spinlock types:

    raw_spinlock_t and spinlock_t

raw_spinlock_t is a real spinning lock independent of CONFIG_PREEMPT_RT,
spinlock_t is mapped to raw_spinlock_t on CONFIG_PREEMPT_RT=n and to a
rtmutex based implementation for CONFIG_PREEMPT_RT=y.

As a consequence spin_lock_irq() and spin_lock_irqsave() will _NOT_
disable interrupts on a CONFIG_PREEMPT_RT=y kernel.

The proposed rust abstraction is not suitable for that.

At this phase of rust integration there is no need to wrap
raw_spinlock_t, so you have two options to solve that:

   1) Map Rust's SpinLockIrq() to spin_lock_irqsave() and
      spin_unlock_irqrestore() which does the right thing

   2) Play all the PREEMPT_RT games in the local irq disable abstraction

#1 is the right thing to do because no driver should rely on actually
disabling interrupts on the CPU. If there is a driver which does that,
then it's not compatible with RT and should use a local lock instead.

local locks aside of being RT compatible have the benefit that they give
scope to the protected region/data, while a plain local_irq_disable()
does not.

Don't even think about exposing this 'with_irq_disabled' interface
unless you are trying to move actual core code like the scheduler or low
level interrupt handling to rust.

Create explicit interrupt safe interfaces which map to the underlying
locking primitives instead.

Thanks,

        tglx