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

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

On 4/27/26 15:32, Vlastimil Babka (SUSE) wrote:
> On 4/16/26 11:10, Harry Yoo (Oracle) wrote:
>> @@ -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?

Oh I forgot about...

> [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;
>> +		}

... this part, so an unconditional allow_spin=false on RT would only fill
the rcu_sheaf once and then stop working.

However, can we condition this on in_nmi() or something, and pretend the
other cases of "unknown context" exist? Installing BPF hooks
allocating/freeing memory into call_rcu() internals sounds crazy, no?

>>  		pcs->rcu_free = NULL;
>>  		rcu_sheaf->node = numa_node_id();
>>  	}
>