mm: slub: init_on_free=1 should wipe freelist ptr for bulk allocations
Subsystems (3)
PATCH 1/2 mm: slub: init_on_free=1 should wipe freelist ptr for bulk allocations
slab_alloc_node() already zeroed out the freelist pointer if init_on_free was on.
Thibaut Sautereau noticed that the same needs to be done for kmem_cache_alloc_bulk(), which performs the allocations separately.
kmem_cache_alloc_bulk() is currently used in two places in the kernel, so this change is unlikely to have a major performance impact.
SLAB doesn't require a similar change, as auto-initialization makes the allocator store the freelist pointers off-slab.
To: Andrew Morton <[email protected]>
To: Christoph Lameter <[email protected]>
Cc: Laura Abbott <[email protected]>
Cc: [email protected]
Cc: [email protected]
--- mm/slub.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-)
mm/slub.c +15 -6
@@ -2669,6 +2669,16 @@ static void *__slab_alloc(struct kmem_cache *s, gfp_t gfpflags, int node,return p; } +/* +* If the object has been wiped upon free, make sure it's fully initialized by +* zeroing out freelist pointer. +*/ +static __always_inline maybe_wipe_obj_freeptr (struct kmem_cache *s,void *obj) +{ +if (unlikely (slab_want_init_on_free (s)) && obj) +memset ((void *)((char *)obj + s->offset),0 ,sizeof (void *)); +} +/* * Inlined fastpath so that allocation functions (kmalloc, kmem_cache_alloc) * have the fastpath folded into their functions. So no function call
@@ -2757,12 +2767,8 @@ static __always_inline void *slab_alloc_node(struct kmem_cache *s,prefetch_freepointer (s, next_object);stat (s,ALLOC_FASTPATH ); } -/* -* If the object has been wiped upon free, make sure it's fully -* initialized by zeroing out freelist pointer. -*/ -if (unlikely (slab_want_init_on_free (s)) && object) -memset (object + s->offset,0 ,sizeof (void *)); + +maybe_wipe_obj_freeptr (s, object);if (unlikely (slab_want_init_on_alloc (gfpflags, s)) && object)memset (object,0 , s->object_size);
@@ -3176,10 +3182,13 @@ int kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, size_t size, goto error; c =this_cpu_ptr (s->cpu_slab); +maybe_wipe_obj_freeptr (s, p[i]); +continue ;/* goto for-loop */ } c->freelist =get_freepointer (s, object); p[i] = object; +maybe_wipe_obj_freeptr (s, p[i]); } c->tid =next_tid (c->tid);local_irq_enable ();
Signature · 1 line
2.23.0.581.g78d2f28ef7-goog
PATCH 2/2 lib/test_meminit: add a kmem_cache_alloc_bulk() test
Make sure allocations from kmem_cache_alloc_bulk()/kmem_cache_free_bulk() are properly initialized.
To: Andrew Morton <[email protected]>
To: Christoph Lameter <[email protected]>
Cc: [email protected]
Cc: [email protected]
--- lib/test_meminit.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+)
lib/test_meminit.c +27 -0
@@ -297,6 +297,32 @@ static int __init do_kmem_cache_rcu_persistent(int size, int *total_failures)return 1 ; } +static int __init do_kmem_cache_size_bulk (int size,int *total_failures) +{ +struct kmem_cache *c; +int i, iter, maxiter =1024 ; +int num, bytes; +bool fail = false; +void *objects[10 ]; + + c =kmem_cache_create ("test_cache" , size, size,0 ,NULL ); +for (iter =0 ; (iter < maxiter) && !fail; iter++) { + num =kmem_cache_alloc_bulk (c,GFP_KERNEL ,ARRAY_SIZE (objects), + objects); +for (i =0 ; i < num; i++) { + bytes =count_nonzero_bytes (objects[i], size); +if (bytes) + fail = true; +fill_with_garbage (objects[i], size); + } + +if (num) +kmem_cache_free_bulk (c, num, objects); + } + *total_failures += fail; +return 1 ; +} +/* * Test kmem_cache allocation by creating caches of different sizes, with and * without constructors, with and without SLAB_TYPESAFE_BY_RCU.
@@ -318,6 +344,7 @@ static int __init test_kmemcache(int *total_failures) num_tests +=do_kmem_cache_size (size, ctor, rcu, zero, &failures); } + num_tests +=do_kmem_cache_size_bulk (size, &failures); }REPORT_FAILURES_IN_FN (); *total_failures += failures;
Signature · 1 line
2.23.0.581.g78d2f28ef7-goog
kbuild test robot
build botI love your patch! Perhaps something to improve:
Hi Alexander,
I love your patch! Perhaps something to improve:
[auto build test WARNING on mmotm/master]
url: https://github.com/0day-ci/linux/commits/Alexander-Potapenko/mm-slub-init_on_free-1-should-wipe-freelist-ptr-for-bulk-allocations/20191005-012134 base: git://git.cmpxchg.org/linux-mmotm.git master config: sparc64-allmodconfig (attached as .config) compiler: sparc64-linux-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=sparc64
If you fix the issue, kindly add following tag
All warnings (new ones prefixed by >>):
>> mm/slub.c:2676:24: warning: return type defaults to 'int' [-Wreturn-type]
static __always_inline maybe_wipe_obj_freeptr(struct kmem_cache *s, void *obj)
^~~~~~~~~~~~~~~~~~~~~~
mm/slub.c: In function 'maybe_wipe_obj_freeptr':
>> mm/slub.c:2680:1: warning: control reaches end of non-void function [-Wreturn-type]
}
^vim +/int +2676 mm/slub.c
2671
2672 /*
2673 * If the object has been wiped upon free, make sure it's fully initialized by
2674 * zeroing out freelist pointer.
2675 */
> 2676 static __always_inline maybe_wipe_obj_freeptr(struct kmem_cache *s, void *obj)
2677 {
2678 if (unlikely(slab_want_init_on_free(s)) && obj)
2679 memset((void *)((char *)obj + s->offset), 0, sizeof(void *));
> 2680 }2681
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation