On 8/24/22 12:24, Hyeonggon Yoo wrote:
On Tue, Aug 23, 2022 at 07:04:00PM +0200, Vlastimil Babka wrote:
The PREEMPT_RT specific disabling of irqs in __cmpxchg_double_slab()
(through slab_[un]lock()) is unnecessary as bit_spin_lock() disables
preemption and that's sufficient on RT where interrupts are threaded.
That means we no longer need the slab_[un]lock() wrappers, so delete
them and rename the current __slab_[un]lock() to slab_[un]lock().
I'm not familiar with PREEMPT_RT preemption model so not sure I'm following.
1) Does "interrupts are threaded on RT" mean processing _most_ (all handlers
that did not specified IRQF_NO_THREAD) of interrupts are delayed to irq threads
and processed later in process context, and the kernel *never* use
spinlock_t, local_lock_t that does not disable interrupts (and sleep) on RT
in hardware/software interrupt context?
AFAIK, yes, that's the case. So if some non-threaded handler used slab,
we would be in trouble. But that would already be the case before this
patch due to the local_lock usage in other paths - the bit_spin_lock()
without disabled irq shouldn't add anything new here AFAIK.
2) Do we need disabling irq in cmpxchg_double_slab() on RT?
By that logic, we don't. But IMHO it's not worth complicating the code
by special casing it for some negligible performance gain (the protected
sections are very short), like we now special case
__cmpxchg_double_slab() for correctness (after this patch, just the
correctness of lockdep_assert_irqs_disabled()).
BTW Is there a good documentation/papers on PREEMPT_RT preemption model?
I tried to find but only found Documentation/locking/locktypes.rst :(
Good question, I don't know myself, maybe the RT guys do.
Thanks!
