Re: [PATCH v2 5/5] mm/slub: simplify __cmpxchg_double_slab() and slab_[un]lock()

[Hyeonggon Yoo <42.hyeyoo@gmail.com>]

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