Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753181Ab2JJG14 (ORCPT ); Wed, 10 Oct 2012 02:27:56 -0400 Received: from mail-wg0-f42.google.com ([74.125.82.42]:62708 "EHLO mail-wg0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752422Ab2JJG1t (ORCPT ); Wed, 10 Oct 2012 02:27:49 -0400 MIME-Version: 1.0 In-Reply-To: References: <20121002170149.GC2465@linux.vnet.ibm.com> <20121002233138.GD2465@linux.vnet.ibm.com> <20121003001530.GF2465@linux.vnet.ibm.com> <0000013a26fb253a-fb5df733-ad41-47c1-af1d-3d6739e417de-000000@email.amazonses.com> <506C52FC.4040305@linux.vnet.ibm.com> Date: Wed, 10 Oct 2012 09:27:47 +0300 X-Google-Sender-Auth: VZOQbCpDROIvTjUPk0kkGJBv664 Message-ID: Subject: Re: [PATCH] [RESEND] mm, slab: release slab_mutex earlier in kmem_cache_destroy() From: Pekka Enberg To: Jiri Kosina Cc: "Srivatsa S. Bhat" , Christoph Lameter , "Paul E. McKenney" , "Paul E. McKenney" , Josh Triplett , linux-kernel@vger.kernel.org, linux-mm@kvack.org, David Rientjes Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6968 Lines: 155 On Mon, Oct 8, 2012 at 10:26 AM, Jiri Kosina wrote: > 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 > --- > mm/slab_common.c | 5 ++++- > 1 files changed, 4 insertions(+), 1 deletions(-) > > diff --git a/mm/slab_common.c b/mm/slab_common.c > index 9c21725..069a24e6 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); Applied, thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/