Skip to content

Commit 3b7b314

Browse files
htejuntorvalds
authored andcommitted
slub: make sysfs file removal asynchronous
Commit bf5eb3d ("slub: separate out sysfs_slab_release() from sysfs_slab_remove()") made slub sysfs file removals synchronous to kmem_cache shutdown. Unfortunately, this created a possible ABBA deadlock between slab_mutex and sysfs draining mechanism triggering the following lockdep warning. ====================================================== [ INFO: possible circular locking dependency detected ] 4.10.0-test+ #48 Not tainted ------------------------------------------------------- rmmod/1211 is trying to acquire lock: (s_active#120){++++.+}, at: [<ffffffff81308073>] kernfs_remove+0x23/0x40 but task is already holding lock: (slab_mutex){+.+.+.}, at: [<ffffffff8120f691>] kmem_cache_destroy+0x41/0x2d0 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (slab_mutex){+.+.+.}: lock_acquire+0xf6/0x1f0 __mutex_lock+0x75/0x950 mutex_lock_nested+0x1b/0x20 slab_attr_store+0x75/0xd0 sysfs_kf_write+0x45/0x60 kernfs_fop_write+0x13c/0x1c0 __vfs_write+0x28/0x120 vfs_write+0xc8/0x1e0 SyS_write+0x49/0xa0 entry_SYSCALL_64_fastpath+0x1f/0xc2 -> #0 (s_active#120){++++.+}: __lock_acquire+0x10ed/0x1260 lock_acquire+0xf6/0x1f0 __kernfs_remove+0x254/0x320 kernfs_remove+0x23/0x40 sysfs_remove_dir+0x51/0x80 kobject_del+0x18/0x50 __kmem_cache_shutdown+0x3e6/0x460 kmem_cache_destroy+0x1fb/0x2d0 kvm_exit+0x2d/0x80 [kvm] vmx_exit+0x19/0xa1b [kvm_intel] SyS_delete_module+0x198/0x1f0 entry_SYSCALL_64_fastpath+0x1f/0xc2 other info that might help us debug this: Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(slab_mutex); lock(s_active#120); lock(slab_mutex); lock(s_active#120); *** DEADLOCK *** 2 locks held by rmmod/1211: #0: (cpu_hotplug.dep_map){++++++}, at: [<ffffffff810a7877>] get_online_cpus+0x37/0x80 #1: (slab_mutex){+.+.+.}, at: [<ffffffff8120f691>] kmem_cache_destroy+0x41/0x2d0 stack backtrace: CPU: 3 PID: 1211 Comm: rmmod Not tainted 4.10.0-test+ #48 Hardware name: Hewlett-Packard HP Compaq Pro 6300 SFF/339A, BIOS K01 v02.05 05/07/2012 Call Trace: print_circular_bug+0x1be/0x210 __lock_acquire+0x10ed/0x1260 lock_acquire+0xf6/0x1f0 __kernfs_remove+0x254/0x320 kernfs_remove+0x23/0x40 sysfs_remove_dir+0x51/0x80 kobject_del+0x18/0x50 __kmem_cache_shutdown+0x3e6/0x460 kmem_cache_destroy+0x1fb/0x2d0 kvm_exit+0x2d/0x80 [kvm] vmx_exit+0x19/0xa1b [kvm_intel] SyS_delete_module+0x198/0x1f0 ? SyS_delete_module+0x5/0x1f0 entry_SYSCALL_64_fastpath+0x1f/0xc2 It'd be the cleanest to deal with the issue by removing sysfs files without holding slab_mutex before the rest of shutdown; however, given the current code structure, it is pretty difficult to do so. This patch punts sysfs file removal to a work item. Before commit bf5eb3d, the removal was punted to a RCU delayed work item which is executed after release. Now, we're punting to a different work item on shutdown which still maintains the goal removing the sysfs files earlier when destroying kmem_caches. Link: http://lkml.kernel.org/r/[email protected] Fixes: bf5eb3d ("slub: separate out sysfs_slab_release() from sysfs_slab_remove()") Signed-off-by: Tejun Heo <[email protected]> Reported-by: Steven Rostedt (VMware) <[email protected]> Tested-by: Steven Rostedt (VMware) <[email protected]> Cc: Christoph Lameter <[email protected]> Cc: Pekka Enberg <[email protected]> Cc: David Rientjes <[email protected]> Cc: Joonsoo Kim <[email protected]> Signed-off-by: Andrew Morton <[email protected]> Signed-off-by: Linus Torvalds <[email protected]>
1 parent a91e0f6 commit 3b7b314

File tree

2 files changed

+27
-14
lines changed

2 files changed

+27
-14
lines changed

include/linux/slub_def.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ struct kmem_cache {
8484
int red_left_pad; /* Left redzone padding size */
8585
#ifdef CONFIG_SYSFS
8686
struct kobject kobj; /* For sysfs */
87+
struct work_struct kobj_remove_work;
8788
#endif
8889
#ifdef CONFIG_MEMCG
8990
struct memcg_cache_params memcg_params;

mm/slub.c

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5625,13 +5625,37 @@ static char *create_unique_id(struct kmem_cache *s)
56255625
return name;
56265626
}
56275627

5628+
static void sysfs_slab_remove_workfn(struct work_struct *work)
5629+
{
5630+
struct kmem_cache *s =
5631+
container_of(work, struct kmem_cache, kobj_remove_work);
5632+
5633+
if (!s->kobj.state_in_sysfs)
5634+
/*
5635+
* For a memcg cache, this may be called during
5636+
* deactivation and again on shutdown. Remove only once.
5637+
* A cache is never shut down before deactivation is
5638+
* complete, so no need to worry about synchronization.
5639+
*/
5640+
return;
5641+
5642+
#ifdef CONFIG_MEMCG
5643+
kset_unregister(s->memcg_kset);
5644+
#endif
5645+
kobject_uevent(&s->kobj, KOBJ_REMOVE);
5646+
kobject_del(&s->kobj);
5647+
kobject_put(&s->kobj);
5648+
}
5649+
56285650
static int sysfs_slab_add(struct kmem_cache *s)
56295651
{
56305652
int err;
56315653
const char *name;
56325654
struct kset *kset = cache_kset(s);
56335655
int unmergeable = slab_unmergeable(s);
56345656

5657+
INIT_WORK(&s->kobj_remove_work, sysfs_slab_remove_workfn);
5658+
56355659
if (!kset) {
56365660
kobject_init(&s->kobj, &slab_ktype);
56375661
return 0;
@@ -5695,20 +5719,8 @@ static void sysfs_slab_remove(struct kmem_cache *s)
56955719
*/
56965720
return;
56975721

5698-
if (!s->kobj.state_in_sysfs)
5699-
/*
5700-
* For a memcg cache, this may be called during
5701-
* deactivation and again on shutdown. Remove only once.
5702-
* A cache is never shut down before deactivation is
5703-
* complete, so no need to worry about synchronization.
5704-
*/
5705-
return;
5706-
5707-
#ifdef CONFIG_MEMCG
5708-
kset_unregister(s->memcg_kset);
5709-
#endif
5710-
kobject_uevent(&s->kobj, KOBJ_REMOVE);
5711-
kobject_del(&s->kobj);
5722+
kobject_get(&s->kobj);
5723+
schedule_work(&s->kobj_remove_work);
57125724
}
57135725

57145726
void sysfs_slab_release(struct kmem_cache *s)

0 commit comments

Comments
 (0)