On Wed, May 5, 2021 at 8:47 AM Waiman Long <longman@xxxxxxxxxx> wrote: > > 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 non-accounted 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 non-accounted > 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. > > 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. > > Suggested-by: Vlastimil Babka <vbabka@xxxxxxx> > Signed-off-by: Waiman Long <longman@xxxxxxxxxx> One nit below and after incorporating Vlastimil's suggestions: Reviewed-by: Shakeel Butt <shakeelb@xxxxxxxxxx> > --- > include/linux/slab.h | 42 ++++++++++++++++++++++++++++++++++-------- > mm/slab_common.c | 23 +++++++++++++++-------- > 2 files changed, 49 insertions(+), 16 deletions(-) > > diff --git a/include/linux/slab.h b/include/linux/slab.h > index 0c97d788762c..f2d9ebc34f5c 100644 > --- a/include/linux/slab.h > +++ b/include/linux/slab.h > @@ -305,9 +305,16 @@ static inline void __check_heap_object(const void *ptr, unsigned long n, > /* > * Whenever changing this, take care of that kmalloc_type() and > * create_kmalloc_caches() still work as intended. > + * > + * KMALLOC_NORMAL is for non-accounted objects only whereas KMALLOC_CGROUP > + * is for accounted objects only. I think you can say "KMALLOC_CGROUP is for accounted and unreclaimable objects only".
