On 8/25/22 03:57, Hyeonggon Yoo wrote:
> With PREEMPT_RT disabling interrupt is unnecessary as there is
> no user of slab in hardirq context on PREEMPT_RT.
>
> The limitation of lockless fastpath on PREEMPT_RT comes from the fact
> that local_lock does not disable preemption on PREEMPT_RT.
>
> Fix comments accordingly.
>
> Signed-off-by: Hyeonggon Yoo <42.hyeyoo@xxxxxxxxx>
Just FTR, as "slub: Make PREEMPT_RT support less convoluted" patch dealt
with these comments already, there's now nothing left to apply from below.
> ---
> mm/slub.c | 23 ++++++++++++++---------
> 1 file changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/mm/slub.c b/mm/slub.c
> index 30c2ee9e8a29..aa42ac6013b8 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -100,7 +100,7 @@
> * except the stat counters. This is a percpu structure manipulated only by
> * the local cpu, so the lock protects against being preempted or interrupted
> * by an irq. Fast path operations rely on lockless operations instead.
> - * On PREEMPT_RT, the local lock does not actually disable irqs (and thus
> + * On PREEMPT_RT, the local lock does not actually disable preemption (and thus
> * prevent the lockless operations), so fastpath operations also need to take
> * the lock and are no longer lockless.
> *
> @@ -3185,10 +3185,12 @@ static __always_inline void *slab_alloc_node(struct kmem_cache *s, struct list_l
> slab = c->slab;
> /*
> * We cannot use the lockless fastpath on PREEMPT_RT because if a
> - * slowpath has taken the local_lock_irqsave(), it is not protected
> - * against a fast path operation in an irq handler. So we need to take
> - * the slow path which uses local_lock. It is still relatively fast if
> - * there is a suitable cpu freelist.
> + * slowpath has taken the local_lock which does not disable preemption
> + * on PREEMPT_RT, it is not protected against a fast path operation in
> + * another thread that does not take the local_lock.
> + *
> + * So we need to take the slow path which uses local_lock. It is still
> + * relatively fast if there is a suitable cpu freelist.
> */
> if (IS_ENABLED(CONFIG_PREEMPT_RT) ||
> unlikely(!object || !slab || !node_match(slab, node))) {
> @@ -3457,10 +3459,13 @@ static __always_inline void do_slab_free(struct kmem_cache *s,
> #else /* CONFIG_PREEMPT_RT */
> /*
> * We cannot use the lockless fastpath on PREEMPT_RT because if
> - * a slowpath has taken the local_lock_irqsave(), it is not
> - * protected against a fast path operation in an irq handler. So
> - * we need to take the local_lock. We shouldn't simply defer to
> - * __slab_free() as that wouldn't use the cpu freelist at all.
> + * a slowpath has taken the local_lock which does not disable
> + * preemption on PREEMPT_RT, it is not protected against a
> + * fast path operation in another thread that does not take
> + * the local_lock.
> + *
> + * So we need to take the local_lock. We shouldn't simply defer
> + * to __slab_free() as that wouldn't use the cpu freelist at all.
> */
> void **freelist;
>
