Skip to content

Commit 079858e

Browse files
0x7f454c46okta-10
authored andcommitted
mm: slab: free kmem_cache_node after destroy sysfs file
When slub_debug alloc_calls_show is enabled we will try to track location and user of slab object on each online node, kmem_cache_node structure and cpu_cache/cpu_slub shouldn't be freed till there is the last reference to sysfs file. This fixes the following panic: BUG: unable to handle kernel NULL pointer dereference at 0000000000000020 IP: list_locations+0x169/0x4e0 PGD 257304067 PUD 438456067 PMD 0 Oops: 0000 [#1] SMP CPU: 3 PID: 973074 Comm: cat ve: 0 Not tainted 3.10.0-229.7.2.ovz.9.30-00007-japdoll-dirty #2 9.30 Hardware name: DEPO Computers To Be Filled By O.E.M./H67DE3, BIOS L1.60c 07/14/2011 task: ffff88042a5dc5b0 ti: ffff88037f8d8000 task.ti: ffff88037f8d8000 RIP: list_locations+0x169/0x4e0 Call Trace: alloc_calls_show+0x1d/0x30 slab_attr_show+0x1b/0x30 sysfs_read_file+0x9a/0x1a0 vfs_read+0x9c/0x170 SyS_read+0x58/0xb0 system_call_fastpath+0x16/0x1b Code: 5e 07 12 00 b9 00 04 00 00 3d 00 04 00 00 0f 4f c1 3d 00 04 00 00 89 45 b0 0f 84 c3 00 00 00 48 63 45 b0 49 8b 9c c4 f8 00 00 00 <48> 8b 43 20 48 85 c0 74 b6 48 89 df e8 46 37 44 00 48 8b 53 10 CR2: 0000000000000020 Separated __kmem_cache_release from __kmem_cache_shutdown which now called on slab_kmem_cache_release (after the last reference to sysfs file object has dropped). Reintroduced locking in free_partial as sysfs file might access cache's partial list after shutdowning - partial revert of the commit 69cb8e6 ("slub: free slabs without holding locks"). Zap __remove_partial and use remove_partial (w/o underscores) as free_partial now takes list_lock which s partial revert for commit 1e4dd94 ("slub: do not assert not having lock in removing freed partial") Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com> Suggested-by: Vladimir Davydov <vdavydov@virtuozzo.com> Acked-by: Vladimir Davydov <vdavydov@virtuozzo.com> Cc: Christoph Lameter <cl@linux.com> Cc: Pekka Enberg <penberg@kernel.org> Cc: David Rientjes <rientjes@google.com> Cc: Joonsoo Kim <iamjoonsoo.kim@lge.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: celtare21 <celtare21@gmail.com> Signed-off-by: Oktapra Amtono <oktapra.amtono@gmail.com>
1 parent a26c598 commit 079858e

File tree

5 files changed

+29
-27
lines changed

5 files changed

+29
-27
lines changed

mm/slab.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2283,7 +2283,7 @@ __kmem_cache_create (struct kmem_cache *cachep, unsigned long flags)
22832283

22842284
err = setup_cpu_cache(cachep, gfp);
22852285
if (err) {
2286-
__kmem_cache_shutdown(cachep);
2286+
__kmem_cache_release(cachep);
22872287
return err;
22882288
}
22892289

@@ -2421,13 +2421,14 @@ int __kmem_cache_shrink(struct kmem_cache *cachep, bool deactivate)
24212421
}
24222422

24232423
int __kmem_cache_shutdown(struct kmem_cache *cachep)
2424+
{
2425+
return __kmem_cache_shrink(cachep, false);
2426+
}
2427+
2428+
void __kmem_cache_release(struct kmem_cache *cachep)
24242429
{
24252430
int i;
24262431
struct kmem_cache_node *n;
2427-
int rc = __kmem_cache_shrink(cachep, false);
2428-
2429-
if (rc)
2430-
return rc;
24312432

24322433
free_percpu(cachep->cpu_cache);
24332434

@@ -2438,7 +2439,6 @@ int __kmem_cache_shutdown(struct kmem_cache *cachep)
24382439
kfree(n);
24392440
cachep->node[i] = NULL;
24402441
}
2441-
return 0;
24422442
}
24432443

24442444
/*

mm/slab.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ static inline unsigned long kmem_cache_flags(unsigned long object_size,
139139
#define CACHE_CREATE_MASK (SLAB_CORE_FLAGS | SLAB_DEBUG_FLAGS | SLAB_CACHE_FLAGS)
140140

141141
int __kmem_cache_shutdown(struct kmem_cache *);
142+
void __kmem_cache_release(struct kmem_cache *);
142143
int __kmem_cache_shrink(struct kmem_cache *, bool);
143144
void slab_kmem_cache_release(struct kmem_cache *);
144145

mm/slab_common.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -698,6 +698,7 @@ static inline int shutdown_memcg_caches(struct kmem_cache *s,
698698

699699
void slab_kmem_cache_release(struct kmem_cache *s)
700700
{
701+
__kmem_cache_release(s);
701702
destroy_memcg_params(s);
702703
kfree_const(s->name);
703704
kmem_cache_free(kmem_cache, s);

mm/slob.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -630,6 +630,10 @@ int __kmem_cache_shutdown(struct kmem_cache *c)
630630
return 0;
631631
}
632632

633+
void __kmem_cache_release(struct kmem_cache *c)
634+
{
635+
}
636+
633637
int __kmem_cache_shrink(struct kmem_cache *d, bool deactivate)
634638
{
635639
return 0;

mm/slub.c

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1656,18 +1656,12 @@ static inline void add_partial(struct kmem_cache_node *n,
16561656
__add_partial(n, page, tail);
16571657
}
16581658

1659-
static inline void
1660-
__remove_partial(struct kmem_cache_node *n, struct page *page)
1661-
{
1662-
list_del(&page->lru);
1663-
n->nr_partial--;
1664-
}
1665-
16661659
static inline void remove_partial(struct kmem_cache_node *n,
16671660
struct page *page)
16681661
{
16691662
lockdep_assert_held(&n->list_lock);
1670-
__remove_partial(n, page);
1663+
list_del(&page->lru);
1664+
n->nr_partial--;
16711665
}
16721666

16731667
/*
@@ -3267,6 +3261,12 @@ static void free_kmem_cache_nodes(struct kmem_cache *s)
32673261
}
32683262
}
32693263

3264+
void __kmem_cache_release(struct kmem_cache *s)
3265+
{
3266+
free_percpu(s->cpu_slab);
3267+
free_kmem_cache_nodes(s);
3268+
}
3269+
32703270
static int init_kmem_cache_nodes(struct kmem_cache *s)
32713271
{
32723272
int node;
@@ -3533,28 +3533,31 @@ static void list_slab_objects(struct kmem_cache *s, struct page *page,
35333533

35343534
/*
35353535
* Attempt to free all partial slabs on a node.
3536-
* This is called from kmem_cache_close(). We must be the last thread
3537-
* using the cache and therefore we do not need to lock anymore.
3536+
* This is called from __kmem_cache_shutdown(). We must take list_lock
3537+
* because sysfs file might still access partial list after the shutdowning.
35383538
*/
35393539
static void free_partial(struct kmem_cache *s, struct kmem_cache_node *n)
35403540
{
35413541
struct page *page, *h;
35423542

3543+
BUG_ON(irqs_disabled());
3544+
spin_lock_irq(&n->list_lock);
35433545
list_for_each_entry_safe(page, h, &n->partial, lru) {
35443546
if (!page->inuse) {
3545-
__remove_partial(n, page);
3547+
remove_partial(n, page);
35463548
discard_slab(s, page);
35473549
} else {
35483550
list_slab_objects(s, page,
3549-
"Objects remaining in %s on kmem_cache_close()");
3551+
"Objects remaining in %s on __kmem_cache_shutdown()");
35503552
}
35513553
}
3554+
spin_unlock_irq(&n->list_lock);
35523555
}
35533556

35543557
/*
35553558
* Release all resources used by a slab cache.
35563559
*/
3557-
static inline int kmem_cache_close(struct kmem_cache *s)
3560+
int __kmem_cache_shutdown(struct kmem_cache *s)
35583561
{
35593562
int node;
35603563
struct kmem_cache_node *n;
@@ -3566,16 +3569,9 @@ static inline int kmem_cache_close(struct kmem_cache *s)
35663569
if (n->nr_partial || slabs_node(s, node))
35673570
return 1;
35683571
}
3569-
free_percpu(s->cpu_slab);
3570-
free_kmem_cache_nodes(s);
35713572
return 0;
35723573
}
35733574

3574-
int __kmem_cache_shutdown(struct kmem_cache *s)
3575-
{
3576-
return kmem_cache_close(s);
3577-
}
3578-
35793575
/********************************************************************
35803576
* Kmalloc subsystem
35813577
*******************************************************************/
@@ -4111,7 +4107,7 @@ int __kmem_cache_create(struct kmem_cache *s, unsigned long flags)
41114107
memcg_propagate_slab_attrs(s);
41124108
err = sysfs_slab_add(s);
41134109
if (err)
4114-
kmem_cache_close(s);
4110+
__kmem_cache_release(s);
41154111

41164112
return err;
41174113
}

0 commit comments

Comments
 (0)