On Wed, Aug 24, 2022 at 01:51:24PM +0200, Vlastimil Babka wrote: > 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. Yeah, that was exactly what I wondered! > 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. Agreed. > > 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()). Okay. Wanted to make sure that disabling interrupts is not required by RT. > > > 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. Okay. Thanks! -- Thanks, Hyeonggon
