Skip to content

Commit 1e4dd94

Browse files
rostedttorvalds
authored andcommitted
slub: do not assert not having lock in removing freed partial
Vladimir reported the following issue: Commit c65c187 ("slub: use lockdep_assert_held") requires remove_partial() to be called with n->list_lock held, but free_partial() called from kmem_cache_close() on cache destruction does not follow this rule, leading to a warning: WARNING: CPU: 0 PID: 2787 at mm/slub.c:1536 __kmem_cache_shutdown+0x1b2/0x1f0() Modules linked in: CPU: 0 PID: 2787 Comm: modprobe Tainted: G W 3.14.0-rc1-mm1+ rib#1 Hardware name: 0000000000000600 ffff88003ae1dde8 ffffffff816d9583 0000000000000600 0000000000000000 ffff88003ae1de28 ffffffff8107c107 0000000000000000 ffff880037ab2b00 ffff88007c240d30 ffffea0001ee5280 ffffea0001ee52a0 Call Trace: __kmem_cache_shutdown+0x1b2/0x1f0 kmem_cache_destroy+0x43/0xf0 xfs_destroy_zones+0x103/0x110 [xfs] exit_xfs_fs+0x38/0x4e4 [xfs] SyS_delete_module+0x19a/0x1f0 system_call_fastpath+0x16/0x1b His solution was to add a spinlock in order to quiet lockdep. Although there would be no contention to adding the lock, that lock also requires disabling of interrupts which will have a larger impact on the system. Instead of adding a spinlock to a location where it is not needed for lockdep, make a __remove_partial() function that does not test if the list_lock is held, as no one should have it due to it being freed. Also added a __add_partial() function that does not do the lock validation either, as it is not needed for the creation of the cache. Signed-off-by: Steven Rostedt <[email protected]> Reported-by: Vladimir Davydov <[email protected]> Suggested-by: David Rientjes <[email protected]> Acked-by: David Rientjes <[email protected]> Acked-by: Vladimir Davydov <[email protected]> Acked-by: Christoph Lameter <[email protected]> Cc: Pekka Enberg <[email protected]> Signed-off-by: Andrew Morton <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
1 parent 25fba9b commit 1e4dd94

File tree

1 file changed

+20
-12
lines changed

1 file changed

+20
-12
lines changed

mm/slub.c

+20-12
Original file line numberDiff line numberDiff line change
@@ -1518,27 +1518,37 @@ static void discard_slab(struct kmem_cache *s, struct page *page)
15181518
/*
15191519
* Management of partially allocated slabs.
15201520
*/
1521-
static inline void add_partial(struct kmem_cache_node *n,
1522-
struct page *page, int tail)
1521+
static inline void
1522+
__add_partial(struct kmem_cache_node *n, struct page *page, int tail)
15231523
{
1524-
lockdep_assert_held(&n->list_lock);
1525-
15261524
n->nr_partial++;
15271525
if (tail == DEACTIVATE_TO_TAIL)
15281526
list_add_tail(&page->lru, &n->partial);
15291527
else
15301528
list_add(&page->lru, &n->partial);
15311529
}
15321530

1533-
static inline void remove_partial(struct kmem_cache_node *n,
1534-
struct page *page)
1531+
static inline void add_partial(struct kmem_cache_node *n,
1532+
struct page *page, int tail)
15351533
{
15361534
lockdep_assert_held(&n->list_lock);
1535+
__add_partial(n, page, tail);
1536+
}
15371537

1538+
static inline void
1539+
__remove_partial(struct kmem_cache_node *n, struct page *page)
1540+
{
15381541
list_del(&page->lru);
15391542
n->nr_partial--;
15401543
}
15411544

1545+
static inline void remove_partial(struct kmem_cache_node *n,
1546+
struct page *page)
1547+
{
1548+
lockdep_assert_held(&n->list_lock);
1549+
__remove_partial(n, page);
1550+
}
1551+
15421552
/*
15431553
* Remove slab from the partial list, freeze it and
15441554
* return the pointer to the freelist.
@@ -2904,12 +2914,10 @@ static void early_kmem_cache_node_alloc(int node)
29042914
inc_slabs_node(kmem_cache_node, node, page->objects);
29052915

29062916
/*
2907-
* the lock is for lockdep's sake, not for any actual
2908-
* race protection
2917+
* No locks need to be taken here as it has just been
2918+
* initialized and there is no concurrent access.
29092919
*/
2910-
spin_lock(&n->list_lock);
2911-
add_partial(n, page, DEACTIVATE_TO_HEAD);
2912-
spin_unlock(&n->list_lock);
2920+
__add_partial(n, page, DEACTIVATE_TO_HEAD);
29132921
}
29142922

29152923
static void free_kmem_cache_nodes(struct kmem_cache *s)
@@ -3195,7 +3203,7 @@ static void free_partial(struct kmem_cache *s, struct kmem_cache_node *n)
31953203

31963204
list_for_each_entry_safe(page, h, &n->partial, lru) {
31973205
if (!page->inuse) {
3198-
remove_partial(n, page);
3206+
__remove_partial(n, page);
31993207
discard_slab(s, page);
32003208
} else {
32013209
list_slab_objects(s, page,

0 commit comments

Comments
 (0)