From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from psmtp.com (na3sys010amx168.postini.com [74.125.245.168]) by kanga.kvack.org (Postfix) with SMTP id 03B806B004D for ; Wed, 11 Jan 2012 12:09:17 -0500 (EST) From: ebiederm@xmission.com (Eric W. Biederman) Subject: Re: [PATCH 1/2] fs: sysfs: Do dcache-related updates to sysfs dentries under sysfs_mutex References: <1326276668-19932-1-git-send-email-mgorman@suse.de> <1326276668-19932-2-git-send-email-mgorman@suse.de> Date: Wed, 11 Jan 2012 09:11:27 -0800 In-Reply-To: <1326276668-19932-2-git-send-email-mgorman@suse.de> (Mel Gorman's message of "Wed, 11 Jan 2012 10:11:07 +0000") Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Sender: owner-linux-mm@kvack.org List-ID: To: Mel Gorman Cc: Linux-MM , Linux-FSDevel , LKML , Andrew Morton , Peter Zijlstra , "Srivatsa S. Bhat" , Russell King - ARM Linux , Gilad Ben-Yossef , "Paul E. McKenney" , Miklos Szeredi , Greg KH , Gong Chen Mel Gorman writes: > While running a CPU hotplug stress test under memory pressure, a > spinlock lockup was detected due to a dentry lock being recursively > taken. When this happens varies considerably and is difficult > to trigger. > > [ 482.345588] BUG: spinlock lockup on CPU#2, udevd/4400 > [ 482.345590] lock: ffff8803075be0d0, .magic: dead4ead, .owner: udevd/5689, .owner_cpu: 0 > [ 482.345592] Pid: 4400, comm: udevd Not tainted 3.2.0-vanilla #1 > [ 482.345592] Call Trace: > [ 482.345595] [] spin_dump+0x88/0x8d > [ 482.345597] [] do_raw_spin_lock+0xd6/0xf9 > [ 482.345599] [] _raw_spin_lock+0x39/0x3d > [ 482.345601] [] ? shrink_dcache_parent+0x77/0x28c > [ 482.345603] [] shrink_dcache_parent+0x77/0x28c > [ 482.345605] [] ? have_submounts+0x13e/0x1bd > [ 482.345607] [] sysfs_dentry_revalidate+0xaa/0xbe > [ 482.345608] [] do_lookup+0x263/0x2fc > [ 482.345610] [] ? security_inode_permission+0x1e/0x20 > [ 482.345612] [] link_path_walk+0x1e2/0x763 > [ 482.345614] [] path_lookupat+0x5c/0x61a > [ 482.345616] [] ? might_fault+0x89/0x8d > [ 482.345618] [] ? might_fault+0x40/0x8d > [ 482.345619] [] do_path_lookup+0x2a/0xa8 > [ 482.345621] [] user_path_at_empty+0x5d/0x97 > [ 482.345623] [] ? trace_hardirqs_off+0xd/0xf > [ 482.345625] [] ? _raw_spin_unlock_irqrestore+0x44/0x5a > [ 482.345627] [] user_path_at+0x11/0x13 > [ 482.345629] [] vfs_fstatat+0x44/0x71 > [ 482.345631] [] vfs_lstat+0x1e/0x20 > [ 482.345632] [] sys_newlstat+0x1f/0x40 > [ 482.345634] [] ? trace_hardirqs_on_caller+0x12d/0x164 > [ 482.345636] [] ? trace_hardirqs_on_thunk+0x3a/0x3f > [ 482.345638] [] ? trace_hardirqs_off+0xd/0xf > [ 482.345640] [] system_call_fastpath+0x16/0x1b > [ 482.515004] [] ? trace_hardirqs_off+0xd/0xf > [ 482.520870] [] system_call_fastpath+0x16/0x1b > > At this point, CPU hotplug stops and other processes get stuck in a > similar deadlock waiting for 5689 to unlock. RCU reports stalls but > it is collateral damage. > > The deadlocked processes have sysfs_dentry_revalidate() in > common. Miklos Szeredi explained at https://lkml.org/lkml/2012/1/9/114 > that the deadlock happens within dcache if two processes call > shrink_dcache_parent() on the same dentry. > > In Miklos's case, the problem is with the bonding driver but during > CPU online or offline, a number of dentries are being created and > deleted and this deadlock is also being hit. Looking at sysfs, there > is a global sysfs_mutex that protects the sysfs directory tree from > concurrent reclaims. Almost all operations involving directory inodes > and dentries take place under the sysfs_mutex - linking, unlinking, > patch searching lookup, renames and readdir. d_invalidate is slightly > different. It is mostly under the mutex but if the dentry has to be > removed from the dcache, the mutex is dropped. The sysfs_mutex protects the sysfs data structures not the vfs. > Where as Miklos' patch changes dcache, this patch changes sysfs to > consistently hold the mutex for dentry-related operations. Once > applied, this particular bug with CPU hotadd/hotremove no longer > occurs. After taking a quick skim over the code to reacquaint myself with it appears that the usage in sysfs is idiomatic. That is sysfs uses shrink_dcache_parent without a lock and in a context where the right race could trigger this deadlock. And in particular I expect you could trigger the same deadlock in proc, nfs, and gfs2 with if you can get the timing right. I don't think adding a work-around for the bug in shrink_dcache_parent is going to do anything except hide the bug in the VFS, and unnecessarily increase the sysfs_mutex hold times. I may be blind but I don't see a reason at this point to rush out an incomplete work-around for the bug in shrink_dcahce_parent instead of actually fixing shrink_dcache_parent. Eric -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: email@kvack.org