Re: [PATCH slab/for-next v4 7/8] mm/slab: introduce kfree_rcu_nolock()

[Vlastimil Babka (SUSE)]

On 7/20/26 14:44, Harry Yoo (Oracle) wrote:
> Currently, k[v]free_rcu() cannot be called in unknown context since
> it could lead to a deadlock when called in the middle of k[v]free_rcu().
> 
> Make users' lives easier by introducing kfree_rcu_nolock() variant,
> now that kfree_rcu_sheaf() is available on PREEMPT_RT and
> __kfree_rcu_sheaf() handles unknown context.
> 
> kfree_rcu_nolock() falls back to the kvfree_rcu batching when sheaves
> path fails.

>From the code I would say it falls back to defer_kfree_rcu(), which is an
irq_work that does kvfree_rcu batching? Otherwise the sentence would
contradict the following:

> In most cases, the sheaves path is expected to succeed
> and it's unnecessary to add complexity to the existing kvfree_rcu
> batching by teaching it how to handle unknown context.

This talks about trying to perform the batching immediately without defering.

> Since defer_kfree_rcu() can be called on caches without sheaves, move
> deferred_work_barrier() and rcu_barrier() outside the branch in
> kvfree_rcu_barrier_on_cache().
> 
> Signed-off-by: Harry Yoo (Oracle) <harry@xxxxxxxxxx>

Other than that and a comment correction below, LGTM.

Reviewed-by: Vlastimil Babka (SUSE) <vbabka@xxxxxxxxxx>

> ---
>  include/linux/rcupdate.h | 19 +++++++++++++++++++
>  mm/slab_common.c         | 28 ++++++++++++++++++++++++++--
>  mm/slub.c                | 24 +++++++++++++++++++++++-
>  3 files changed, 68 insertions(+), 3 deletions(-)
> 
> diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> index ef5bb6981133..ce0fd401352e 100644
> --- a/include/linux/rcupdate.h
> +++ b/include/linux/rcupdate.h
> @@ -1099,6 +1099,7 @@ static inline void rcu_read_unlock_migrate(void)
>   * In mm/slab_common.c, no suitable header to include here.
>   */
>  void kvfree_call_rcu(struct kvfree_rcu_head *head, void *ptr);
> +void kfree_call_rcu_nolock(struct kvfree_rcu_head *head, void *ptr);
>  
>  /*
>   * The BUILD_BUG_ON() makes sure the rcu_head offset can be handled. See the
> @@ -1124,6 +1125,24 @@ do {								\
>  		kvfree_call_rcu(NULL, (void *) (___p));		\
>  } while (0)
>  
> +/**
> + * kfree_rcu_nolock() - a version of kfree_rcu() that can be called in any context.
> + * @ptr: pointer to kfree for double-argument invocations.
> + * @kvrhf: the name of the struct kvfree_rcu_head within the type of @ptr.
> + *
> + * Objects that are not allocated via kmalloc_nolock() are not supported.

Is this true? If __kfree_rcu_sheaf() succeeds, rcu_sheaf is either flushed
normally (call_rcu with rcu_free_sheaf()) or via the irq_work doing the
same. The defer_kfree_rcu() fallback ends up doing a normal
kvfree_call_rcu(). I don't see any path that ends up doing kfree_nolock().
Am I missing something?

> + * kfree_rcu_nolock() supports 2-arg variant only.
> + */
> +#define kfree_rcu_nolock(ptr, kvrhf)						\
> +do {										\
> +	typeof (ptr) ___p = (ptr);						\
> +										\
> +	if (___p) {								\
> +		BUILD_BUG_ON(offsetof(typeof(*(ptr)), kvrhf) >= 4096);		\
> +		kfree_call_rcu_nolock(&((___p)->kvrhf), (void *) (___p));	\
> +	}									\
> +} while (0)
> +
>  /*
>   * Place this after a lock-acquisition primitive to guarantee that
>   * an UNLOCK+LOCK pair acts as a full barrier.  This guarantee applies