2006-10-18 03:59:43

by Srinivasa Ds

[permalink] [raw]
Subject: Issues with possible recursive locking

When I was removing dlm module,I hit in to below error.

==========================================
[ INFO: possible recursive locking detected ]

2.6.18#1
---------------------------------------------
modprobe/4501 is trying to acquire lock:
(&inode->i_mutex){--..}, at: [<c0611e5a>] mutex_lock+0x21/0x24

but task is already holding lock:
(&inode->i_mutex){--..}, at: [<c0611e5a>] mutex_lock+0x21/0x24

other info that might help us debug this:
1 lock held by modprobe/4501:
#0: (&inode->i_mutex){--..}, at: [<c0611e5a>] mutex_lock+0x21/0x24

stack backtrace:
[<c04051ed>] show_trace_log_lvl+0x58/0x16a
[<c04057fa>] show_trace+0xd/0x10
[<c0405913>] dump_stack+0x19/0x1b
[<c043b6f1>] __lock_acquire+0x778/0x99c
[<c043be86>] lock_acquire+0x4b/0x6d
[<c0611ceb>] __mutex_lock_slowpath+0xbc/0x20a
[<c0611e5a>] mutex_lock+0x21/0x24
[<f89c2562>] configfs_unregister_subsystem+0x3e/0xa8 [configfs]
[<f8f4263f>] dlm_config_exit+0xd/0xf [dlm]
[<f8f4db94>] exit_dlm+0x12/0x23 [dlm]
[<c0442790>] sys_delete_module+0x18d/0x1b5
[<c0403fb7>] syscall_call+0x7/0xb
===========================================================
Cause for this problem is, lock-validator validates the locks through
lock class. And by definition,a lock in struct inode considered as one
class, irrespective of number of of instances of different inode present
in the system.
Hence 2 consecutive mutex lock on d_inode->i_mutex considered as
recursive lock,eventhough both inodes are different. Thats what
happening below. Is it not a kernel design constraint ??

==============================================
void configfs_unregister_subsystem(struct configfs_subsystem *subsys)
{
struct config_group *group = &subsys->su_group;
struct dentry *dentry = group->cg_item.ci_dentry;

if (dentry->d_parent != configfs_sb->s_root) {
printk(KERN_ERR "configfs: Tried to unregister
non-subsystem!\n");
return;
}

mutex_lock(&configfs_sb->s_root->d_inode->i_mutex);
mutex_lock(&dentry->d_inode->i_mutex);
==> problem is here
if (configfs_detach_prep(dentry)) {
printk(KERN_ERR "configfs: Tried to unregister non-empty
subsystem!\n");
}
===========================================



2006-10-19 16:53:59

by Mark Fasheh

[permalink] [raw]
Subject: Re: Issues with possible recursive locking

On Wed, Oct 18, 2006 at 09:37:58AM +0530, Srinivasa Ds wrote:
> When I was removing dlm module,I hit in to below error.
This patch should take care of that particular warning, please let me know
if it doesn't. I'll carry it in ocfs2.git shortly.

Hmm, I get other warnings from configfs starting and stopping the ocfs2
cluster stack, so I bet we've got some more mutex_lock() calls in there to
change to mutex_lock_nested():

[ INFO: possible recursive locking detected ]
2.6.19-rc2 #1
---------------------------------------------
o2cb_ctl/2457 is trying to acquire lock:
(&inode->i_mutex){--..}, at: [<c02ff984>] mutex_lock+0x21/0x24

but task is already holding lock:
(&inode->i_mutex){--..}, at: [<c02ff984>] mutex_lock+0x21/0x24

other info that might help us debug this:
2 locks held by o2cb_ctl/2457:
#0: (&inode->i_mutex/1){--..}, at: [<c0177194>] lookup_create+0x1d/0x73
#1: (&inode->i_mutex){--..}, at: [<c02ff984>] mutex_lock+0x21/0x24

stack backtrace:
[<c0104d0a>] dump_trace+0x64/0x1c2
[<c0104e7a>] show_trace_log_lvl+0x12/0x25
[<c01053c6>] show_trace+0xd/0x10
[<c01054dc>] dump_stack+0x19/0x1b
[<c013c7bb>] __lock_acquire+0x6c6/0x8e3
[<c013cf1b>] lock_acquire+0x4b/0x6c
[<c02ff81d>] __mutex_lock_slowpath+0xb0/0x1f6
[<c02ff984>] mutex_lock+0x21/0x24
[<f8aa2800>] configfs_add_file+0x36/0x60 [configfs]
[<f8aa285f>] configfs_create_file+0x35/0x38 [configfs]
[<f8aa3260>] configfs_attach_item+0x13d/0x180 [configfs]
[<f8aa32b7>] configfs_attach_group+0x14/0x154 [configfs]
[<f8aa3377>] configfs_attach_group+0xd4/0x154 [configfs]
[<f8aa3d8b>] configfs_mkdir+0x1b2/0x287 [configfs]
[<c017666a>] vfs_mkdir+0xca/0x131
[<c0178c8d>] sys_mkdirat+0x88/0xbb
[<c0178cd0>] sys_mkdir+0x10/0x12
[<c0103e2b>] syscall_call+0x7/0xb
--Mark


configfs: mutex_lock_nested() fix

configfs_unregister_subsystem() nests a pair of inode i_mutex acquisitions,
and thus needs annotation via mutex_lock_nested().

Signed-off-by: Mark Fasheh <[email protected]>

diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index 8a3b6a1..452cfd1 100644
--- a/fs/configfs/dir.c
+++ b/fs/configfs/dir.c
@@ -1176,8 +1176,9 @@ void configfs_unregister_subsystem(struc
return;
}

- mutex_lock(&configfs_sb->s_root->d_inode->i_mutex);
- mutex_lock(&dentry->d_inode->i_mutex);
+ mutex_lock_nested(&configfs_sb->s_root->d_inode->i_mutex,
+ I_MUTEX_PARENT);
+ mutex_lock_nested(&dentry->d_inode->i_mutex, I_MUTEX_CHILD);
if (configfs_detach_prep(dentry)) {
printk(KERN_ERR "configfs: Tried to unregister non-empty subsystem!\n");
}

2006-10-25 12:04:37

by Srinivasa Ds

[permalink] [raw]
Subject: Re: Issues with possible recursive locking

Mark Fasheh wrote:
> On Wed, Oct 18, 2006 at 09:37:58AM +0530, Srinivasa Ds wrote:
>
>> When I was removing dlm module,I hit in to below error.
>>
> This patch should take care of that particular warning, please let me know
> if it doesn't. I'll carry it in ocfs2.git shortly.
>
Thanks Mark,It worked fine for me.
> Hmm, I get other warnings from configfs starting and stopping the ocfs2
> cluster stack, so I bet we've got some more mutex_lock() calls in there to
> change to mutex_lock_nested():
>
> [ INFO: possible recursive locking detected ]
> 2.6.19-rc2 #1
> ---------------------------------------------
> o2cb_ctl/2457 is trying to acquire lock:
> (&inode->i_mutex){--..}, at: [<c02ff984>] mutex_lock+0x21/0x24
>
> but task is already holding lock:
> (&inode->i_mutex){--..}, at: [<c02ff984>] mutex_lock+0x21/0x24
>
> other info that might help us debug this:
> 2 locks held by o2cb_ctl/2457:
> #0: (&inode->i_mutex/1){--..}, at: [<c0177194>] lookup_create+0x1d/0x73
> #1: (&inode->i_mutex){--..}, at: [<c02ff984>] mutex_lock+0x21/0x24
>
> stack backtrace:
> [<c0104d0a>] dump_trace+0x64/0x1c2
> [<c0104e7a>] show_trace_log_lvl+0x12/0x25
> [<c01053c6>] show_trace+0xd/0x10
> [<c01054dc>] dump_stack+0x19/0x1b
> [<c013c7bb>] __lock_acquire+0x6c6/0x8e3
> [<c013cf1b>] lock_acquire+0x4b/0x6c
> [<c02ff81d>] __mutex_lock_slowpath+0xb0/0x1f6
> [<c02ff984>] mutex_lock+0x21/0x24
> [<f8aa2800>] configfs_add_file+0x36/0x60 [configfs]
> [<f8aa285f>] configfs_create_file+0x35/0x38 [configfs]
> [<f8aa3260>] configfs_attach_item+0x13d/0x180 [configfs]
> [<f8aa32b7>] configfs_attach_group+0x14/0x154 [configfs]
> [<f8aa3377>] configfs_attach_group+0xd4/0x154 [configfs]
> [<f8aa3d8b>] configfs_mkdir+0x1b2/0x287 [configfs]
> [<c017666a>] vfs_mkdir+0xca/0x131
> [<c0178c8d>] sys_mkdirat+0x88/0xbb
> [<c0178cd0>] sys_mkdir+0x10/0x12
> [<c0103e2b>] syscall_call+0x7/0xb
> --Mark
>
>
> configfs: mutex_lock_nested() fix
>
> configfs_unregister_subsystem() nests a pair of inode i_mutex acquisitions,
> and thus needs annotation via mutex_lock_nested().
>
> Signed-off-by: Mark Fasheh <[email protected]>
>
> diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
> index 8a3b6a1..452cfd1 100644
> --- a/fs/configfs/dir.c
> +++ b/fs/configfs/dir.c
> @@ -1176,8 +1176,9 @@ void configfs_unregister_subsystem(struc
> return;
> }
>
> - mutex_lock(&configfs_sb->s_root->d_inode->i_mutex);
> - mutex_lock(&dentry->d_inode->i_mutex);
> + mutex_lock_nested(&configfs_sb->s_root->d_inode->i_mutex,
> + I_MUTEX_PARENT);
> + mutex_lock_nested(&dentry->d_inode->i_mutex, I_MUTEX_CHILD);
> if (configfs_detach_prep(dentry)) {
> printk(KERN_ERR "configfs: Tried to unregister non-empty subsystem!\n");
> }
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
>