All of lore.kernel.org
 help / color / mirror / Atom feed
From: Uladzislau Rezki <urezki@gmail.com>
To: Vlastimil Babka <vbabka@suse.cz>
Cc: Christoph Lameter <cl@linux.com>,
	David Rientjes <rientjes@google.com>,
	"Paul E. McKenney" <paulmck@kernel.org>,
	Joel Fernandes <joel@joelfernandes.org>,
	Josh Triplett <josh@joshtriplett.org>,
	Boqun Feng <boqun.feng@gmail.com>,
	Uladzislau Rezki <urezki@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Roman Gushchin <roman.gushchin@linux.dev>,
	Hyeonggon Yoo <42.hyeyoo@gmail.com>,
	linux-mm@kvack.org, Steven Rostedt <rostedt@goodmis.org>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Lai Jiangshan <jiangshanlai@gmail.com>,
	Zqiang <qiang.zhang1211@gmail.com>,
	rcu@vger.kernel.org
Subject: Re: [PATCH RFC 4/4] slab: don't batch kvfree_rcu() with SLUB_TINY
Date: Fri, 24 Jan 2025 13:11:01 +0100	[thread overview]
Message-ID: <Z5ODVa3osRqNFlX6@pc636> (raw)
In-Reply-To: <20250123-slub-tiny-kfree_rcu-v1-4-0e386ef1541a@suse.cz>

On Thu, Jan 23, 2025 at 11:37:21AM +0100, Vlastimil Babka wrote:
> kvfree_rcu() is batched for better performance except on TINY_RCU, which
> is a simple implementation for small UP systems. Similarly SLUB_TINY is
> an option intended for small systems, whether or not used together with
> TINY_RCU. In case SLUB_TINY is used with !TINY_RCU, it makes arguably
> sense to not do the batching and limit the memory footprint. It's also
> suboptimal to have RCU-specific #ifdefs in slab code.
> 
> With that, add CONFIG_KFREE_RCU_BATCHED to determine whether batching
> kvfree_rcu() implementation is used. It is not set by a user prompt, but
> enabled by default and disabled in case TINY_RCU or SLUB_TINY are
> enabled.
> 
> Use the new config for #ifdef's in slab code and extend their scope to
> cover all code used by the batched kvfree_rcu(). For example there's no
> need to perform kvfree_rcu_init() if the batching is disabled.
> 
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> ---
>  include/linux/slab.h |  2 +-
>  mm/Kconfig           |  4 ++++
>  mm/slab_common.c     | 45 +++++++++++++++++++++++++--------------------
>  3 files changed, 30 insertions(+), 21 deletions(-)
> 
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index bcc62e5656c35c6a3f4caf26fb33d7447dead39a..9faf33734a8eee2425b90e679c0457ab459422a3 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -1083,7 +1083,7 @@ extern void kvfree_sensitive(const void *addr, size_t len);
>  
>  unsigned int kmem_cache_size(struct kmem_cache *s);
>  
> -#ifdef CONFIG_TINY_RCU
> +#ifndef CONFIG_KFREE_RCU_BATCHED
>  static inline void kvfree_rcu_barrier(void)
>  {
>  	rcu_barrier();
> diff --git a/mm/Kconfig b/mm/Kconfig
> index 84000b01680869801a10f56f06d0c43d6521a8d2..e513308a4aed640ee556ecb5793c7f3f195bbcae 100644
> --- a/mm/Kconfig
> +++ b/mm/Kconfig
> @@ -242,6 +242,10 @@ menu "Slab allocator options"
>  config SLUB
>  	def_bool y
>  
> +config KFREE_RCU_BATCHED
> +	def_bool y
> +	depends on !SLUB_TINY && !TINY_RCU
> +
>  config SLUB_TINY
>  	bool "Configure for minimal memory footprint"
>  	depends on EXPERT
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index f13d2c901daf1419993620459fbd5845eecb85f1..9f6d66313afc6684bdc0f32908fe01c83c60f283 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -1284,6 +1284,28 @@ EXPORT_TRACEPOINT_SYMBOL(kmem_cache_alloc);
>  EXPORT_TRACEPOINT_SYMBOL(kfree);
>  EXPORT_TRACEPOINT_SYMBOL(kmem_cache_free);
>  
> +#ifndef CONFIG_KFREE_RCU_BATCHED
> +
> +void kvfree_call_rcu(struct rcu_head *head, void *ptr)
> +{
> +	if (head) {
> +		kasan_record_aux_stack_noalloc(ptr);
> +		call_rcu(head, kvfree_rcu_cb);
> +		return;
> +	}
> +
> +	// kvfree_rcu(one_arg) call.
> +	might_sleep();
> +	synchronize_rcu();
> +	kvfree(ptr);
> +}
> +
> +void __init kvfree_rcu_init(void)
> +{
> +}
> +
> +#else /* CONFIG_KFREE_RCU_BATCHED */
> +
>  /*
>   * This rcu parameter is runtime-read-only. It reflects
>   * a minimum allowed number of objects which can be cached
> @@ -1858,24 +1880,6 @@ add_ptr_to_bulk_krc_lock(struct kfree_rcu_cpu **krcp,
>  	return true;
>  }
>  
> -#ifdef CONFIG_TINY_RCU
> -
> -void kvfree_call_rcu(struct rcu_head *head, void *ptr)
> -{
> -	if (head) {
> -		kasan_record_aux_stack_noalloc(ptr);
> -		call_rcu(head, kvfree_rcu_cb);
> -		return;
> -	}
> -
> -	// kvfree_rcu(one_arg) call.
> -	might_sleep();
> -	synchronize_rcu();
> -	kvfree(ptr);
> -}
> -
> -#else /* !CONFIG_TINY_RCU */
> -
>  static enum hrtimer_restart
>  schedule_page_work_fn(struct hrtimer *t)
>  {
> @@ -2084,8 +2088,6 @@ void kvfree_rcu_barrier(void)
>  }
>  EXPORT_SYMBOL_GPL(kvfree_rcu_barrier);
>  
> -#endif /* !CONFIG_TINY_RCU */
> -
>  static unsigned long
>  kfree_rcu_shrink_count(struct shrinker *shrink, struct shrink_control *sc)
>  {
> @@ -2175,3 +2177,6 @@ void __init kvfree_rcu_init(void)
>  
>  	shrinker_register(kfree_rcu_shrinker);
>  }
> +
> +#endif /* CONFIG_KFREE_RCU_BATCHED */
> +
> 
> -- 
> 2.48.1
> 
Reviewed-by: Uladzislau Rezki (Sony) <urezki@gmail.com>

A small nit: CONFIG_KVFREE_RCU_BATCHED?

--
Uladzislau Rezki

  parent reply	other threads:[~2025-01-24 12:11 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-23 10:37 [PATCH RFC 0/4] slab, rcu: move and consolidate TINY_RCU kvfree_rcu() to SLAB Vlastimil Babka
2025-01-23 10:37 ` [PATCH RFC 1/4] slab, rcu: move TINY_RCU variant of " Vlastimil Babka
2025-01-24 12:56   ` Uladzislau Rezki
2025-01-23 10:37 ` [PATCH RFC 2/4] rcu: remove trace_rcu_kvfree_callback Vlastimil Babka
2025-01-23 10:37 ` [PATCH RFC 3/4] rcu, slab: use a regular callback function for kvfree_rcu Vlastimil Babka
2025-01-24 12:47   ` Uladzislau Rezki
2025-01-24 14:16     ` Vlastimil Babka
2025-01-24 14:19       ` Uladzislau Rezki
2025-01-23 10:37 ` [PATCH RFC 4/4] slab: don't batch kvfree_rcu() with SLUB_TINY Vlastimil Babka
2025-01-24  2:43   ` kernel test robot
2025-01-24 12:11   ` Uladzislau Rezki [this message]
2025-01-23 19:37 ` [PATCH RFC 0/4] slab, rcu: move and consolidate TINY_RCU kvfree_rcu() to SLAB Joel Fernandes
2025-01-23 20:26 ` Paul E. McKenney
2025-01-24 14:25 ` Vlastimil Babka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z5ODVa3osRqNFlX6@pc636 \
    --to=urezki@gmail.com \
    --cc=42.hyeyoo@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=boqun.feng@gmail.com \
    --cc=cl@linux.com \
    --cc=jiangshanlai@gmail.com \
    --cc=joel@joelfernandes.org \
    --cc=josh@joshtriplett.org \
    --cc=linux-mm@kvack.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=paulmck@kernel.org \
    --cc=qiang.zhang1211@gmail.com \
    --cc=rcu@vger.kernel.org \
    --cc=rientjes@google.com \
    --cc=roman.gushchin@linux.dev \
    --cc=rostedt@goodmis.org \
    --cc=vbabka@suse.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.