Re: [PATCH 5/8] mm/slab: make kfree_rcu_nolock() work with sheaves

["Vlastimil Babka (SUSE)" <vbabka@kernel.org>]

On 4/16/26 11:10, Harry Yoo (Oracle) wrote:
> Teach kfree_rcu_sheaf() how to handle the !allow_spin case. Similar to
> __pcs_replace_full_main(), try to get an empty sheaf from pcs->spare or
> the barn, but don't add !allow_spin support for alloc_empty_sheaf() and
> fail early instead.
> 
> Since call_rcu() does not support NMI contexts, kfree_rcu_sheaf() fails
> when the rcu sheaf becomes full.
> 
> Signed-off-by: Harry Yoo (Oracle) <harry@kernel.org>
> ---
>  mm/slab.h        |  2 +-
>  mm/slab_common.c |  7 +++----
>  mm/slub.c        | 14 ++++++++++++--
>  3 files changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/mm/slab.h b/mm/slab.h
> index ae2e990e8dc2..d7fd7626e9fe 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -409,7 +409,7 @@ static inline bool is_kmalloc_normal(struct kmem_cache *s)
>  	return !(s->flags & (SLAB_CACHE_DMA|SLAB_ACCOUNT|SLAB_RECLAIM_ACCOUNT));
>  }
>  
> -bool __kfree_rcu_sheaf(struct kmem_cache *s, void *obj);
> +bool __kfree_rcu_sheaf(struct kmem_cache *s, void *obj, bool allow_spin);
>  void flush_all_rcu_sheaves(void);
>  void flush_rcu_sheaves_on_cache(struct kmem_cache *s);
>  void defer_kvfree_rcu_barrier(void);
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index e840956233dd..46a2bee1662b 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -1716,7 +1716,7 @@ static void kfree_rcu_work(struct work_struct *work)
>  		kvfree_rcu_list(head);
>  }
>  
> -static bool kfree_rcu_sheaf(void *obj)
> +static bool kfree_rcu_sheaf(void *obj, bool allow_spin)
>  {
>  	struct kmem_cache *s;
>  	struct slab *slab;
> @@ -1730,7 +1730,7 @@ static bool kfree_rcu_sheaf(void *obj)
>  
>  	s = slab->slab_cache;
>  	if (likely(!IS_ENABLED(CONFIG_NUMA) || slab_nid(slab) == numa_mem_id()))
> -		return __kfree_rcu_sheaf(s, obj);
> +		return __kfree_rcu_sheaf(s, obj, allow_spin);
>  
>  	return false;
>  }
> @@ -2111,8 +2111,7 @@ void kvfree_call_rcu_ptr(struct rcu_ptr *head, void *ptr, bool allow_spin)
>  				IS_ENABLED(CONFIG_DEBUG_KMEMLEAK)))
>  		goto defer_free;
>  
> -	if (!IS_ENABLED(CONFIG_PREEMPT_RT) &&
> -			(allow_spin && kfree_rcu_sheaf(ptr)))
> +	if (!IS_ENABLED(CONFIG_PREEMPT_RT) && kfree_rcu_sheaf(ptr, allow_spin))
>  		return;

I wonder if this patch is enough to drop (or reduce a lot) the PREEMPT_RT
incompatibility? It IIRC came from Ulad's review point [1] that since RT
spinlock is in fact a mutex, it's not unsafe to take it with e.g. disabled
preemption, and it's also why kfree_rcu_cpu uses a raw_spin_lock_t, etc.

But with kfree_rcu_sheaf now having allow_spin parameter and only doing a
trylock at most with that false, it should now be safe to allow it for
CONFIG_PREEMPT_RT, only pass allow_spin = false unconditionally there?

[1] https://lore.kernel.org/all/aL_uhPtztx7Ef0T2@pc636/

>  
>  	// Queue the object but don't yet schedule the batch.
> diff --git a/mm/slub.c b/mm/slub.c
> index 6f658ec00751..d0db8d070570 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -5895,7 +5895,7 @@ static void rcu_free_sheaf(struct rcu_head *head)
>   */
>  static DEFINE_WAIT_OVERRIDE_MAP(kfree_rcu_sheaf_map, LD_WAIT_CONFIG);
>  
> -bool __kfree_rcu_sheaf(struct kmem_cache *s, void *obj)
> +bool __kfree_rcu_sheaf(struct kmem_cache *s, void *obj, bool allow_spin)
>  {
>  	struct slub_percpu_sheaves *pcs;
>  	struct slab_sheaf *rcu_sheaf;
> @@ -5933,7 +5933,7 @@ bool __kfree_rcu_sheaf(struct kmem_cache *s, void *obj)
>  			goto fail;
>  		}
>  
> -		empty = barn_get_empty_sheaf(barn, true);
> +		empty = barn_get_empty_sheaf(barn, allow_spin);
>  
>  		if (empty) {
>  			pcs->rcu_free = empty;
> @@ -5942,6 +5942,10 @@ bool __kfree_rcu_sheaf(struct kmem_cache *s, void *obj)
>  
>  		local_unlock(&s->cpu_sheaves->lock);
>  
> +		/* It's easier to fall back than trying harder with !allow_spin */
> +		if (!allow_spin)
> +			goto fail;
> +
>  		empty = alloc_empty_sheaf(s, GFP_NOWAIT);
>  
>  		if (!empty)
> @@ -5973,6 +5977,12 @@ bool __kfree_rcu_sheaf(struct kmem_cache *s, void *obj)
>  	if (likely(rcu_sheaf->size < s->sheaf_capacity)) {
>  		rcu_sheaf = NULL;
>  	} else {
> +		if (unlikely(!allow_spin)) {
> +			/* call_rcu() cannot be called in an unknown context */
> +			rcu_sheaf->size--;
> +			local_unlock(&s->cpu_sheaves->lock);
> +			goto fail;
> +		}
>  		pcs->rcu_free = NULL;
>  		rcu_sheaf->node = numa_node_id();
>  	}