Attribute removal patch causes lockdep warning

Attribute removal patch causes lockdep warning

[Alan Stern]

Oliver:

Are you aware that your patch for safe attribute file removal provokes a 
lockdep warning at bootup?

[   25.270416] =============================================
[   25.270518] [ INFO: possible recursive locking detected ]
[   25.270571] 2.6.20-rc4 #3
[   25.270619] ---------------------------------------------
[   25.270671] init/1 is trying to acquire lock:
[   25.270722]  (&sysfs_inode_imutex_key){--..}, at: [<c0293d72>] mutex_lock+0x1c/0x1f
[   25.270932] 
[   25.270934] but task is already holding lock:
[   25.271028]  (&sysfs_inode_imutex_key){--..}, at: [<c0293d72>] mutex_lock+0x1c/0x1f
[   25.271233] 
[   25.271234] other info that might help us debug this:
[   25.271330] 2 locks held by init/1:
[   25.271378]  #0:  (tty_mutex){--..}, at: [<c0293d72>] mutex_lock+0x1c/0x1f
[   25.271607]  #1:  (&sysfs_inode_imutex_key){--..}, at: [<c0293d72>] mutex_lock+0x1c/0x1f
[   25.271847] 
[   25.271848] stack backtrace:
[   25.271940]  [<c01040ce>] show_trace_log_lvl+0x1a/0x2f
[   25.272032]  [<c0104781>] show_trace+0x12/0x14
[   25.272121]  [<c0104833>] dump_stack+0x16/0x18
[   25.272208]  [<c01376b9>] __lock_acquire+0x116/0x9fd
[   25.272298]  [<c0138289>] lock_acquire+0x56/0x6f
[   25.272385]  [<c0293bc9>] __mutex_lock_slowpath+0xe5/0x272
[   25.272473]  [<c0293d72>] mutex_lock+0x1c/0x1f
[   25.272559]  [<c018e9ff>] sysfs_drop_dentry+0xb7/0x12d
[   25.272648]  [<c018eb40>] sysfs_hash_and_remove+0x8c/0x12e
[   25.272737]  [<c019080a>] sysfs_remove_link+0xb/0xd
[   25.272824]  [<c0213025>] device_del+0x96/0x1f4
[   25.272913]  [<c021318e>] device_unregister+0xb/0x15
[   25.273000]  [<c0213225>] device_destroy+0x8d/0x93
[   25.273086]  [<c01ffa60>] vcs_remove_sysfs+0x1a/0x36
[   25.273175]  [<c0204765>] con_close+0x4c/0x60
[   25.273263]  [<c01f8f72>] release_dev+0x221/0x631
[   25.273349]  [<c01f9394>] tty_release+0x12/0x1c
[   25.273435]  [<c0160b77>] __fput+0xb9/0x155
[   25.273522]  [<c0160c2a>] fput+0x17/0x19
[   25.273607]  [<c015e618>] filp_close+0x54/0x5c
[   25.273693]  [<c015f633>] sys_close+0x78/0xb0
[   25.273779]  [<c010309e>] sysenter_past_esp+0x5f/0x99
[   25.273866]  =======================

This has been the case for quite a while.  As near as I can tell, closing
a console device calls orphan_all_buffers() while holding an inode mutex.  
Since that routine acquires an inode mutex, there is a potential for
deadlock.

This isn't meant to imply that your patch is wrong.  Actually I think the 
patch is good and should be kept; it ought to be possible to resolve the 
warning by changing something else -- although I don't know what.  Do you 
see anything in the console or vc driver that could be fixed?

Alan Stern

Re: Attribute removal patch causes lockdep warning

[Oliver Neukum]

Am Dienstag, 16. Januar 2007 21:33 schrieb Alan Stern:
> Are you aware that your patch for safe attribute file removal provokes a 
> lockdep warning at bootup?

Yes, I am aware of that. However, the top down lock order is always
followed. A patch to make the lock checker realize that has been posted
and included upstream.

	Regards
		Oliver

Re: Attribute removal patch causes lockdep warning

[Greg KH]

On Tue, Jan 16, 2007 at 10:15:57PM +0100, Oliver Neukum wrote:
> Am Dienstag, 16. Januar 2007 21:33 schrieb Alan Stern:
> > Are you aware that your patch for safe attribute file removal provokes a 
> > lockdep warning at bootup?
> 
> Yes, I am aware of that. However, the top down lock order is always
> followed. A patch to make the lock checker realize that has been posted
> and included upstream.


Alan, here's the patch:


Signed-off-by: Frederik Deweerdt <frederik.deweerdt@gmail.com>

diff --git a/fs/sysfs/inode.c b/fs/sysfs/inode.c
index 8c533cb..3b5574b 100644
--- a/fs/sysfs/inode.c
+++ b/fs/sysfs/inode.c
@@ -214,7 +214,7 @@ static inline void orphan_all_buffers(st
 	struct sysfs_buffer_collection *set = node->i_private;
 	struct sysfs_buffer *buf;
 
-	mutex_lock(&node->i_mutex);
+	mutex_lock_nested(&node->i_mutex, I_MUTEX_CHILD);
 	if (node->i_private) {
 		list_for_each_entry(buf, &set->associates, associates) {
 			down(&buf->sem);
@@ -271,7 +271,7 @@ int sysfs_hash_and_remove(struct dentry
 		return -ENOENT;
 
 	parent_sd = dir->d_fsdata;
-	mutex_lock(&dir->d_inode->i_mutex);
+	mutex_lock_nested(&dir->d_inode->i_mutex, I_MUTEX_PARENT);
 	list_for_each_entry(sd, &parent_sd->s_children, s_sibling) {
 		if (!sd->s_element)
 			continue;