diff mbox series

[v2] SLUB: Add support for per object memory policies

Message ID 20240906-strict_numa-v2-1-f104e6de6d1e@gentwo.org (mailing list archive)
State New
Headers show
Series [v2] SLUB: Add support for per object memory policies | expand

Commit Message

Christoph Lameter via B4 Relay Sept. 6, 2024, 4:04 p.m. UTC 
From: Christoph Lameter <cl@gentwo.org>

    The old SLAB allocator used to support memory policies on a per
    allocation bases. In SLUB the memory policies are applied on a
    per page frame / folio bases. Doing so avoids having to check memory
    policies in critical code paths for kmalloc and friends.

    This worked on general well on Intel/AMD/PowerPC because the
    interconnect technology is mature and can minimize the latencies
    through intelligent caching even if a small object is not
    placed optimally.

    However, on ARM we have an emergence of new NUMA interconnect
    technology based more on embedded devices. Caching of remote content
    can currently be ineffective using the standard building blocks / mesh
    available on that platform. Such architectures benefit if each slab
    object is individually placed according to memory policies
    and other restrictions.

    This patch adds another kernel parameter

            slab_strict_numa

    If that is set then a static branch is activated that will cause
    the hotpaths of the allocator to evaluate the current memory
    allocation policy. Each object will be properly placed by
    paying the price of extra processing and SLUB will no longer
    defer to the page allocator to apply memory policies at the
    folio level.

    This patch improves performance of memcached running
    on Ampere Altra 2P system (ARM Neoverse N1 processor)
    by 3.6% due to accurate placement of small kernel objects.

Tested-by: Huang Shijie <shijie@os.amperecomputing.com>
Signed-off-by: Christoph Lameter (Ampere) <cl@gentwo.org>
---
Changes in v2:
- Fix various issues
- Testing
---
 mm/slub.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)


---
base-commit: b831f83e40a24f07c8dcba5be408d93beedc820f
change-id: 20240819-strict_numa-fc59b33123a2

Best regards,

Comments

kernel test robot Sept. 7, 2024, 7:55 p.m. UTC | #1 
Hi Christoph,

kernel test robot noticed the following build warnings:

[auto build test WARNING on b831f83e40a24f07c8dcba5be408d93beedc820f]

url:    https://github.com/intel-lab-lkp/linux/commits/Christoph-Lameter-via-B4-Relay/SLUB-Add-support-for-per-object-memory-policies/20240907-000537
base:   b831f83e40a24f07c8dcba5be408d93beedc820f
patch link:    https://lore.kernel.org/r/20240906-strict_numa-v2-1-f104e6de6d1e%40gentwo.org
patch subject: [PATCH v2] SLUB: Add support for per object memory policies
config: sparc64-randconfig-r121-20240907 (https://download.01.org/0day-ci/archive/20240908/202409080304.haF25cFZ-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 14.1.0
reproduce: (https://download.01.org/0day-ci/archive/20240908/202409080304.haF25cFZ-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409080304.haF25cFZ-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> mm/slub.c:222:1: sparse: sparse: symbol 'strict_numa' was not declared. Should it be static?
   mm/slub.c: note: in included file (through include/linux/smp.h, include/linux/lockdep.h, include/linux/spinlock.h, ...):
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
   mm/slub.c:3036:55: sparse: sparse: context imbalance in '__put_partials' - unexpected unlock
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
   mm/slub.c:4327:47: sparse: sparse: context imbalance in '__slab_free' - unexpected unlock
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true

vim +/strict_numa +222 mm/slub.c

   220	
   221	#ifdef CONFIG_NUMA
 > 222	DEFINE_STATIC_KEY_FALSE(strict_numa);
   223	#endif
   224
Christoph Lameter (Ampere) Sept. 12, 2024, 4:15 p.m. UTC | #2 
On Sun, 8 Sep 2024, kernel test robot wrote:

> config: sparc64-randconfig-r121-20240907 (https://download.01.org/0day-ci/archive/20240908/202409080304.haF25cFZ-lkp@intel.com/config)
> compiler: sparc64-linux-gcc (GCC) 14.1.0
> reproduce: (https://download.01.org/0day-ci/archive/20240908/202409080304.haF25cFZ-lkp@intel.com/reproduce)

> sparse warnings: (new ones prefixed by >>)
> >> mm/slub.c:222:1: sparse: sparse: symbol 'strict_numa' was not declared. Should it be static?


Ummm.. This code declares strict_numa. Whats wrong with sparc64 / sparse ?

> vim +/strict_numa +222 mm/slub.c
>
>    220
>    221	#ifdef CONFIG_NUMA
>  > 222	DEFINE_STATIC_KEY_FALSE(strict_numa);
>    223	#endif
>    224
>
Matthew Wilcox Sept. 12, 2024, 5:28 p.m. UTC | #3 
On Thu, Sep 12, 2024 at 09:15:34AM -0700, Christoph Lameter (Ampere) wrote:
> > sparse warnings: (new ones prefixed by >>)
> > >> mm/slub.c:222:1: sparse: sparse: symbol 'strict_numa' was not declared. Should it be static?
> 
> 
> Ummm.. This code declares strict_numa. Whats wrong with sparc64 / sparse ?
> 
> > vim +/strict_numa +222 mm/slub.c
> >
> >    220
> >    221	#ifdef CONFIG_NUMA
> >  > 222	DEFINE_STATIC_KEY_FALSE(strict_numa);

maybe this should be:

static DEFINE_STATIC_KEY_FALSE(strict_numa);

if it's only used within mm/slub.c?  Or it needs to be declared in a
header file if it is used outside mm/slub.c.

> >    223	#endif
> >    224
> >
> 
>
Christoph Lameter (Ampere) Sept. 12, 2024, 10:21 p.m. UTC | #4 
On Thu, 12 Sep 2024, Matthew Wilcox wrote:

> On Thu, Sep 12, 2024 at 09:15:34AM -0700, Christoph Lameter (Ampere) wrote:
> > > sparse warnings: (new ones prefixed by >>)
> > > >> mm/slub.c:222:1: sparse: sparse: symbol 'strict_numa' was not declared. Should it be static?
> >
> >
> > Ummm.. This code declares strict_numa. Whats wrong with sparc64 / sparse ?
> >
> > > vim +/strict_numa +222 mm/slub.c
> > >
> > >    220
> > >    221	#ifdef CONFIG_NUMA
> > >  > 222	DEFINE_STATIC_KEY_FALSE(strict_numa);
>
> maybe this should be:
>
> static DEFINE_STATIC_KEY_FALSE(strict_numa);

Ah double staticity. Thanks.




From b239f4f26094845af74b091682f0bdcae56b5123 Mon Sep 17 00:00:00 2001
From: Christoph Lameter <cl@gentwo.org>
Date: Thu, 12 Sep 2024 15:15:07 -0700
Subject: [PATCH 3/3] [SLUB] Make strict_numa static

strict_numa is only used in mm/slub.c so it can be static.

Signed-off-by: Christoph Lameter <cl@linux.com>
---
 mm/slub.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/slub.c b/mm/slub.c
index 2fa7c35e076a..56e320082c09 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -219,7 +219,7 @@ DEFINE_STATIC_KEY_FALSE(slub_debug_enabled);
 #endif		/* CONFIG_SLUB_DEBUG */

 #ifdef CONFIG_NUMA
-DEFINE_STATIC_KEY_FALSE(strict_numa);
+static DEFINE_STATIC_KEY_FALSE(strict_numa);
 #endif

 /* Structure holding parameters for get_partial() call chain */
diff mbox series

Patch

diff --git a/mm/slub.c b/mm/slub.c
index a77f354f8325..2fa7c35e076a 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -218,6 +218,10 @@  DEFINE_STATIC_KEY_FALSE(slub_debug_enabled);
 #endif
 #endif		/* CONFIG_SLUB_DEBUG */
 
+#ifdef CONFIG_NUMA
+DEFINE_STATIC_KEY_FALSE(strict_numa);
+#endif
+
 /* Structure holding parameters for get_partial() call chain */
 struct partial_context {
 	gfp_t flags;
@@ -3865,6 +3869,28 @@  static __always_inline void *__slab_alloc_node(struct kmem_cache *s,
 	object = c->freelist;
 	slab = c->slab;
 
+#ifdef CONFIG_NUMA
+	if (static_branch_unlikely(&strict_numa) &&
+			node == NUMA_NO_NODE) {
+
+		struct mempolicy *mpol = current->mempolicy;
+
+		if (mpol) {
+			/*
+			 * Special BIND rule support. If existing slab
+			 * is in permitted set then do not redirect
+			 * to a particular node.
+			 * Otherwise we apply the memory policy to get
+			 * the node we need to allocate on.
+			 */
+			if (mpol->mode != MPOL_BIND || !slab ||
+					!node_isset(slab_nid(slab), mpol->nodes))
+
+				node = mempolicy_slab_node();
+		}
+	}
+#endif
+
 	if (!USE_LOCKLESS_FAST_PATH() ||
 	    unlikely(!object || !slab || !node_match(slab, node))) {
 		object = __slab_alloc(s, gfpflags, node, addr, c, orig_size);
@@ -5527,6 +5553,22 @@  static int __init setup_slub_min_objects(char *str)
 __setup("slab_min_objects=", setup_slub_min_objects);
 __setup_param("slub_min_objects=", slub_min_objects, setup_slub_min_objects, 0);
 
+#ifdef CONFIG_NUMA
+static int __init setup_slab_strict_numa(char *str)
+{
+	if (nr_node_ids > 1) {
+		static_branch_enable(&strict_numa);
+		printk(KERN_INFO "SLUB: Strict NUMA enabled.\n");
+	} else
+		printk(KERN_WARNING "slab_strict_numa parameter set on non NUMA system.\n");
+
+	return 1;
+}
+
+__setup("slab_strict_numa", setup_slab_strict_numa);
+#endif
+
+
 #ifdef CONFIG_HARDENED_USERCOPY
 /*
  * Rejects incorrectly sized objects and objects that are to be copied