From cbdd786b817a6be48a630239090e340e1e0bc6b5 Mon Sep 17 00:00:00 2001 From: Jiri Kosina Date: Fri, 12 Oct 2012 15:22:45 +1100 Subject: [PATCH] mm, slab: release slab_mutex earlier in kmem_cache_destroy() Commit 1331e7a1bbe1 ("rcu: Remove _rcu_barrier() dependency on __stop_machine()") introduced slab_mutex -> cpu_hotplug.lock dependency through kmem_cache_destroy() -> rcu_barrier() -> _rcu_barrier() -> get_online_cpus(). Lockdep thinks that this might actually result in ABBA deadlock, and reports it as below: === [ cut here ] === ====================================================== [ INFO: possible circular locking dependency detected ] 3.6.0-rc5-00004-g0d8ee37 #143 Not tainted ------------------------------------------------------- kworker/u:2/40 is trying to acquire lock: (rcu_sched_state.barrier_mutex){+.+...}, at: [] _rcu_barrier+0x26/0x1e0 but task is already holding lock: (slab_mutex){+.+.+.}, at: [] kmem_cache_destroy+0x45/0xe0 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (slab_mutex){+.+.+.}: [] validate_chain+0x632/0x720 [] __lock_acquire+0x309/0x530 [] lock_acquire+0x121/0x190 [] __mutex_lock_common+0x5c/0x450 [] mutex_lock_nested+0x3e/0x50 [] cpuup_callback+0x2f/0xbe [] notifier_call_chain+0x93/0x140 [] __raw_notifier_call_chain+0x9/0x10 [] _cpu_up+0xba/0x14e [] cpu_up+0xbc/0x117 [] smp_init+0x6b/0x9f [] kernel_init+0x147/0x1dc [] kernel_thread_helper+0x4/0x10 -> #1 (cpu_hotplug.lock){+.+.+.}: [] validate_chain+0x632/0x720 [] __lock_acquire+0x309/0x530 [] lock_acquire+0x121/0x190 [] __mutex_lock_common+0x5c/0x450 [] mutex_lock_nested+0x3e/0x50 [] get_online_cpus+0x37/0x50 [] _rcu_barrier+0xbb/0x1e0 [] rcu_barrier_sched+0x10/0x20 [] rcu_barrier+0x9/0x10 [] deactivate_locked_super+0x49/0x90 [] deactivate_super+0x61/0x70 [] mntput_no_expire+0x127/0x180 [] sys_umount+0x6e/0xd0 [] system_call_fastpath+0x16/0x1b -> #0 (rcu_sched_state.barrier_mutex){+.+...}: [] check_prev_add+0x3de/0x440 [] validate_chain+0x632/0x720 [] __lock_acquire+0x309/0x530 [] lock_acquire+0x121/0x190 [] __mutex_lock_common+0x5c/0x450 [] mutex_lock_nested+0x3e/0x50 [] _rcu_barrier+0x26/0x1e0 [] rcu_barrier_sched+0x10/0x20 [] rcu_barrier+0x9/0x10 [] kmem_cache_destroy+0xd1/0xe0 [] nf_conntrack_cleanup_net+0xe4/0x110 [nf_conntrack] [] nf_conntrack_cleanup+0x2a/0x70 [nf_conntrack] [] nf_conntrack_net_exit+0x5e/0x80 [nf_conntrack] [] ops_exit_list+0x39/0x60 [] cleanup_net+0xfb/0x1b0 [] process_one_work+0x26b/0x4c0 [] worker_thread+0x12e/0x320 [] kthread+0x9e/0xb0 [] kernel_thread_helper+0x4/0x10 other info that might help us debug this: Chain exists of: rcu_sched_state.barrier_mutex --> cpu_hotplug.lock --> slab_mutex Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(slab_mutex); lock(cpu_hotplug.lock); lock(slab_mutex); lock(rcu_sched_state.barrier_mutex); *** DEADLOCK *** === [ cut here ] === This is actually a false positive. Lockdep has no way of knowing the fact that the ABBA can actually never happen, because of special semantics of cpu_hotplug.refcount and its handling in cpu_hotplug_begin(); the mutual exclusion there is not achieved through mutex, but through cpu_hotplug.refcount. The "neither cpu_up() nor cpu_down() will proceed past cpu_hotplug_begin() until everyone who called get_online_cpus() will call put_online_cpus()" semantics is totally invisible to lockdep. This patch therefore moves the unlock of slab_mutex so that rcu_barrier() is being called with it unlocked. It has two advantages: - it slightly reduces hold time of slab_mutex; as it's used to protect the cachep list, it's not necessary to hold it over kmem_cache_free() call any more - it silences the lockdep false positive warning, as it avoids lockdep ever learning about slab_mutex -> cpu_hotplug.lock dependency Reviewed-by: Paul E. McKenney Reviewed-by: Srivatsa S. Bhat Acked-by: David Rientjes Signed-off-by: Jiri Kosina Cc: Pekka Enberg Cc: Signed-off-by: Andrew Morton --- mm/slab_common.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/mm/slab_common.c b/mm/slab_common.c index 9c217255ac49..069a24e64403 100644 --- a/mm/slab_common.c +++ b/mm/slab_common.c @@ -168,6 +168,7 @@ void kmem_cache_destroy(struct kmem_cache *s) list_del(&s->list); if (!__kmem_cache_shutdown(s)) { + mutex_unlock(&slab_mutex); if (s->flags & SLAB_DESTROY_BY_RCU) rcu_barrier(); @@ -175,12 +176,14 @@ void kmem_cache_destroy(struct kmem_cache *s) kmem_cache_free(kmem_cache, s); } else { list_add(&s->list, &slab_caches); + mutex_unlock(&slab_mutex); printk(KERN_ERR "kmem_cache_destroy %s: Slab cache still has objects\n", s->name); dump_stack(); } + } else { + mutex_unlock(&slab_mutex); } - mutex_unlock(&slab_mutex); put_online_cpus(); } EXPORT_SYMBOL(kmem_cache_destroy); -- 2.39.5