Re: [-mm patch] lockdep: possible deadlock in sysfs

[Oliver Neukum <oliver@neukum.org>]

Am Freitag, 5. Januar 2007 13:16 schrieb Frederik Deweerdt:
> On Thu, Jan 04, 2007 at 10:02:00PM -0800, Andrew Morton wrote:
> > 
> > 	ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.20-rc3/2.6.20-rc3-mm1/
> > 
> Hi,
> 
> Lockdep issues the following warning:
> [    9.064000] =============================================
> [    9.064000] [ INFO: possible recursive locking detected ]
> [    9.064000] 2.6.20-rc3-mm1 #3
> [    9.064000] ---------------------------------------------
> [    9.064000] init/1 is trying to acquire lock:
> [    9.064000]  (&sysfs_inode_imutex_key){--..}, at: [<c03e6afc>] mutex_lock+0x1c/0x1f
> [    9.064000]
> [    9.064000] but task is already holding lock:
> [    9.064000]  (&sysfs_inode_imutex_key){--..}, at: [<c03e6afc>] mutex_lock+0x1c/0x1f
> [    9.065000]
> [    9.065000] other info that might help us debug this:
> [    9.065000] 2 locks held by init/1:
> [    9.065000]  #0:  (tty_mutex){--..}, at: [<c03e6afc>] mutex_lock+0x1c/0x1f
> [    9.065000]  #1:  (&sysfs_inode_imutex_key){--..}, at: [<c03e6afc>] mutex_lock+0x1c/0x1f
> [    9.065000]
> [    9.065000] stack backtrace:
> [    9.065000]  [<c010390d>] show_trace_log_lvl+0x1a/0x30
> [    9.066000]  [<c0103935>] show_trace+0x12/0x14
> [    9.066000]  [<c0103a2f>] dump_stack+0x16/0x18
> [    9.066000]  [<c0138cb8>] print_deadlock_bug+0xb9/0xc3
> [    9.066000]  [<c0138d17>] check_deadlock+0x55/0x5a
> [    9.066000]  [<c013a953>] __lock_acquire+0x371/0xbf0
> [    9.066000]  [<c013b7a9>] lock_acquire+0x69/0x83
> [    9.066000]  [<c03e6b7e>] __mutex_lock_slowpath+0x75/0x2d1
> [    9.066000]  [<c03e6afc>] mutex_lock+0x1c/0x1f
> [    9.066000]  [<c01b249c>] sysfs_drop_dentry+0xb1/0x133
> [    9.066000]  [<c01b25d1>] sysfs_hash_and_remove+0xb3/0x142
> [    9.066000]  [<c01b30ed>] sysfs_remove_file+0xd/0x10
> [    9.067000]  [<c02849e0>] device_remove_file+0x23/0x2e
> [    9.067000]  [<c02850b2>] device_del+0x188/0x1e6
> [    9.067000]  [<c028511b>] device_unregister+0xb/0x15
> [    9.067000]  [<c0285318>] device_destroy+0x9c/0xa9
> [    9.067000]  [<c0261431>] vcs_remove_sysfs+0x1c/0x3b
> [    9.067000]  [<c0267a08>] con_close+0x5e/0x6b
> [    9.067000]  [<c02598f2>] release_dev+0x4c4/0x6e5
> [    9.067000]  [<c0259faa>] tty_release+0x12/0x1c
> [    9.067000]  [<c0174872>] __fput+0x177/0x1a0
> [    9.067000]  [<c01746f5>] fput+0x3b/0x41
> [    9.068000]  [<c0172ee1>] filp_close+0x36/0x65
> [    9.068000]  [<c0172f73>] sys_close+0x63/0xa4
> [    9.068000]  [<c0102a96>] sysenter_past_esp+0x5f/0x99
> [    9.068000]  =======================
> 
> This is due to sysfs_hash_and_remove() holding dir->d_inode->i_mutex
> before calling sysfs_drop_dentry() which calls orphan_all_buffers()
> which in turn takes node->i_mutex.
> The following patch solves the problem by defering the buffers orphaning
> after the dir->d_inode->imutex is released. Not sure it's the best
> solution though, better wait for feedback from Maneesh and Oliver.

Hi,

are you sure there's a code path that takes these locks in the reverse order?
I've looked through the code twice and not found any. It doesn't make much
sense to first lock the file and afterwards the directory.

Regarding your patch, it should work, but I don't see the need for it.

	Regards
		Oliver