Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

mm: memcg/slab: create a new set of kmalloc-cg-<n> caches

There are currently two problems in the way the objcg pointer array
(memcg_data) in the page structure is being allocated and freed.

On its allocation, it is possible that the allocated objcg pointer
array comes from the same slab that requires memory accounting. If this
happens, the slab will never become empty again as there is at least
one object left (the obj_cgroup array) in the slab.

When it is freed, the objcg pointer array object may be the last one
in its slab and hence causes kfree() to be called again. With the
right workload, the slab cache may be set up in a way that allows the
recursive kfree() calling loop to nest deep enough to cause a kernel
stack overflow and panic the system.

One way to solve this problem is to split the kmalloc-<n> caches
(KMALLOC_NORMAL) into two separate sets - a new set of kmalloc-<n>
(KMALLOC_NORMAL) caches for unaccounted objects only and a new set of
kmalloc-cg-<n> (KMALLOC_CGROUP) caches for accounted objects only. All
the other caches can still allow a mix of accounted and unaccounted
objects.

With this change, all the objcg pointer array objects will come from
KMALLOC_NORMAL caches which won't have their objcg pointer arrays. So
both the recursive kfree() problem and non-freeable slab problem are
gone.

Since both the KMALLOC_NORMAL and KMALLOC_CGROUP caches no longer have
mixed accounted and unaccounted objects, this will slightly reduce the
number of objcg pointer arrays that need to be allocated and save a bit
of memory. On the other hand, creating a new set of kmalloc caches does
have the effect of reducing cache utilization. So it is properly a wash.

The new KMALLOC_CGROUP is added between KMALLOC_NORMAL and
KMALLOC_RECLAIM so that the first for loop in create_kmalloc_caches()
will include the newly added caches without change.

[vbabka@suse.cz: don't create kmalloc-cg caches with cgroup.memory=nokmem]
Link: https://lkml.kernel.org/r/20210512145107.6208-1-longman@redhat.com
[akpm@linux-foundation.org: un-fat-finger v5 delta creation]
[longman@redhat.com: disable cache merging for KMALLOC_NORMAL caches]
Link: https://lkml.kernel.org/r/20210505200610.13943-4-longman@redhat.com

Link: https://lkml.kernel.org/r/20210512145107.6208-1-longman@redhat.com
Link: https://lkml.kernel.org/r/20210505200610.13943-3-longman@redhat.com
Signed-off-by: Waiman Long <longman@redhat.com>
Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
Suggested-by: Vlastimil Babka <vbabka@suse.cz>
Reviewed-by: Shakeel Butt <shakeelb@google.com>
Acked-by: Roman Gushchin <guro@fb.com>
Cc: Christoph Lameter <cl@linux.com>
Cc: David Rientjes <rientjes@google.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com>
Cc: Michal Hocko <mhocko@kernel.org>
Cc: Pekka Enberg <penberg@kernel.org>
Cc: Vladimir Davydov <vdavydov.dev@gmail.com>
[longman@redhat.com: fix for CONFIG_ZONE_DMA=n]
Suggested-by: Roman Gushchin <guro@fb.com>
Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
Cc: Vladimir Davydov <vdavydov.dev@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

authored by

Waiman Long and committed by
Linus Torvalds
494c1dfe 41eb5df1

+62 -19
+33 -9
include/linux/slab.h
··· 305 305 /* 306 306 * Whenever changing this, take care of that kmalloc_type() and 307 307 * create_kmalloc_caches() still work as intended. 308 + * 309 + * KMALLOC_NORMAL can contain only unaccounted objects whereas KMALLOC_CGROUP 310 + * is for accounted but unreclaimable and non-dma objects. All the other 311 + * kmem caches can have both accounted and unaccounted objects. 308 312 */ 309 313 enum kmalloc_cache_type { 310 314 KMALLOC_NORMAL = 0, 315 + #ifndef CONFIG_ZONE_DMA 316 + KMALLOC_DMA = KMALLOC_NORMAL, 317 + #endif 318 + #ifndef CONFIG_MEMCG_KMEM 319 + KMALLOC_CGROUP = KMALLOC_NORMAL, 320 + #else 321 + KMALLOC_CGROUP, 322 + #endif 311 323 KMALLOC_RECLAIM, 312 324 #ifdef CONFIG_ZONE_DMA 313 325 KMALLOC_DMA, ··· 331 319 extern struct kmem_cache * 332 320 kmalloc_caches[NR_KMALLOC_TYPES][KMALLOC_SHIFT_HIGH + 1]; 333 321 322 + /* 323 + * Define gfp bits that should not be set for KMALLOC_NORMAL. 324 + */ 325 + #define KMALLOC_NOT_NORMAL_BITS \ 326 + (__GFP_RECLAIMABLE | \ 327 + (IS_ENABLED(CONFIG_ZONE_DMA) ? __GFP_DMA : 0) | \ 328 + (IS_ENABLED(CONFIG_MEMCG_KMEM) ? __GFP_ACCOUNT : 0)) 329 + 334 330 static __always_inline enum kmalloc_cache_type kmalloc_type(gfp_t flags) 335 331 { 336 - #ifdef CONFIG_ZONE_DMA 337 332 /* 338 333 * The most common case is KMALLOC_NORMAL, so test for it 339 - * with a single branch for both flags. 334 + * with a single branch for all the relevant flags. 340 335 */ 341 - if (likely((flags & (__GFP_DMA | __GFP_RECLAIMABLE)) == 0)) 336 + if (likely((flags & KMALLOC_NOT_NORMAL_BITS) == 0)) 342 337 return KMALLOC_NORMAL; 343 338 344 339 /* 345 - * At least one of the flags has to be set. If both are, __GFP_DMA 346 - * is more important. 340 + * At least one of the flags has to be set. Their priorities in 341 + * decreasing order are: 342 + * 1) __GFP_DMA 343 + * 2) __GFP_RECLAIMABLE 344 + * 3) __GFP_ACCOUNT 347 345 */ 348 - return flags & __GFP_DMA ? KMALLOC_DMA : KMALLOC_RECLAIM; 349 - #else 350 - return flags & __GFP_RECLAIMABLE ? KMALLOC_RECLAIM : KMALLOC_NORMAL; 351 - #endif 346 + if (IS_ENABLED(CONFIG_ZONE_DMA) && (flags & __GFP_DMA)) 347 + return KMALLOC_DMA; 348 + if (!IS_ENABLED(CONFIG_MEMCG_KMEM) || (flags & __GFP_RECLAIMABLE)) 349 + return KMALLOC_RECLAIM; 350 + else 351 + return KMALLOC_CGROUP; 352 352 } 353 353 354 354 /*
+5
mm/internal.h
··· 116 116 extern pmd_t *mm_find_pmd(struct mm_struct *mm, unsigned long address); 117 117 118 118 /* 119 + * in mm/memcontrol.c: 120 + */ 121 + extern bool cgroup_memory_nokmem; 122 + 123 + /* 119 124 * in mm/page_alloc.c 120 125 */ 121 126
+1 -1
mm/memcontrol.c
··· 83 83 static bool cgroup_memory_nosocket; 84 84 85 85 /* Kernel memory accounting disabled? */ 86 - static bool cgroup_memory_nokmem; 86 + bool cgroup_memory_nokmem; 87 87 88 88 /* Whether the swap controller is active */ 89 89 #ifdef CONFIG_MEMCG_SWAP
+23 -9
mm/slab_common.c
··· 738 738 } 739 739 740 740 #ifdef CONFIG_ZONE_DMA 741 - #define INIT_KMALLOC_INFO(__size, __short_size) \ 742 - { \ 743 - .name[KMALLOC_NORMAL] = "kmalloc-" #__short_size, \ 744 - .name[KMALLOC_RECLAIM] = "kmalloc-rcl-" #__short_size, \ 745 - .name[KMALLOC_DMA] = "dma-kmalloc-" #__short_size, \ 746 - .size = __size, \ 747 - } 741 + #define KMALLOC_DMA_NAME(sz) .name[KMALLOC_DMA] = "dma-kmalloc-" #sz, 748 742 #else 743 + #define KMALLOC_DMA_NAME(sz) 744 + #endif 745 + 746 + #ifdef CONFIG_MEMCG_KMEM 747 + #define KMALLOC_CGROUP_NAME(sz) .name[KMALLOC_CGROUP] = "kmalloc-cg-" #sz, 748 + #else 749 + #define KMALLOC_CGROUP_NAME(sz) 750 + #endif 751 + 749 752 #define INIT_KMALLOC_INFO(__size, __short_size) \ 750 753 { \ 751 754 .name[KMALLOC_NORMAL] = "kmalloc-" #__short_size, \ 752 755 .name[KMALLOC_RECLAIM] = "kmalloc-rcl-" #__short_size, \ 756 + KMALLOC_CGROUP_NAME(__short_size) \ 757 + KMALLOC_DMA_NAME(__short_size) \ 753 758 .size = __size, \ 754 759 } 755 - #endif 756 760 757 761 /* 758 762 * kmalloc_info[] is to make slub_debug=,kmalloc-xx option work at boot time. ··· 842 838 static void __init 843 839 new_kmalloc_cache(int idx, enum kmalloc_cache_type type, slab_flags_t flags) 844 840 { 845 - if (type == KMALLOC_RECLAIM) 841 + if (type == KMALLOC_RECLAIM) { 846 842 flags |= SLAB_RECLAIM_ACCOUNT; 843 + } else if (IS_ENABLED(CONFIG_MEMCG_KMEM) && (type == KMALLOC_CGROUP)) { 844 + if (cgroup_memory_nokmem) { 845 + kmalloc_caches[type][idx] = kmalloc_caches[KMALLOC_NORMAL][idx]; 846 + return; 847 + } 848 + flags |= SLAB_ACCOUNT; 849 + } 847 850 848 851 kmalloc_caches[type][idx] = create_kmalloc_cache( 849 852 kmalloc_info[idx].name[type], ··· 868 857 int i; 869 858 enum kmalloc_cache_type type; 870 859 860 + /* 861 + * Including KMALLOC_CGROUP if CONFIG_MEMCG_KMEM defined 862 + */ 871 863 for (type = KMALLOC_NORMAL; type <= KMALLOC_RECLAIM; type++) { 872 864 for (i = KMALLOC_SHIFT_LOW; i <= KMALLOC_SHIFT_HIGH; i++) { 873 865 if (!kmalloc_caches[type][i])