diff for duplicates of <4836F48A.70008@kerlabs.com> diff --git a/a/1.txt b/N1/1.txt index 505b49c..42fcd4b 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -1,4 +1,4 @@ -Louis Rilling a ?crit : +Louis Rilling a écrit : > When destroying a config_group having multiple (nested or not) default groups, > lockdep raises a warning because multiple locks of class I_MUTEX_NORMAL are > taken in configfs_detach_prep(). @@ -64,10 +64,3 @@ Dr Louis Rilling Kerlabs Skype: louis.rilling Batiment Germanium Phone: (+33|0) 6 80 89 08 23 80 avenue des Buttes de Coesmes http://www.kerlabs.com/ 35700 Rennes --------------- next part -------------- -A non-text attachment was scrubbed... -Name: configfs-make-default-group-destruction-lockdep-friendly-2.patch -Type: text/x-diff -Size: 7137 bytes -Desc: not available -Url : http://oss.oracle.com/pipermail/ocfs2-devel/attachments/20080523/53738fb6/attachment.bin diff --git a/N1/2.hdr b/N1/2.hdr new file mode 100644 index 0000000..ff74405 --- /dev/null +++ b/N1/2.hdr @@ -0,0 +1,5 @@ +Content-Type: text/x-diff; name*0="configfs-make-default-group-destruction-lockdep-friendly-2.patch"; charset=iso-8859-1 +Content-Transfer-Encoding: quoted-printable +Content-Disposition: inline; + filename*0="configfs-make-default-group-destruction-lockdep-friendly-2.p"; + filename*1="atch" diff --git a/N1/2.txt b/N1/2.txt new file mode 100644 index 0000000..75f860c --- /dev/null +++ b/N1/2.txt @@ -0,0 +1,205 @@ +configfs: Make multiple default_group destructions lockdep friendly + +When destroying a config_group having multiple (nested or not) default groups, +lockdep raises a warning because multiple locks of class I_MUTEX_NORMAL are +taken in configfs_detach_prep(). + +This patch makes such default group destructions lockdep-friendly by increasing +the i_mutex sub-class from I_MUTEX_CHILD + 1 onwards for each default group +locked under a being-destroyed config_group. + +With this patch and lockdep compiled-in, the number of default groups (whatever +their depth in the tree) under a user-created config_group (resp. registered +subsystem) is limited to MAX_LOCKDEP_SUBCLASSES - I_MUTEX_CHILD - 1 == 3. This +limit is removed when not compiling lockdep. + +============================================= +[ INFO: possible recursive locking detected ] +2.6.26-rc3 #5 +--------------------------------------------- +rmdir/1499 is trying to acquire lock: + (&sb->s_type->i_mutex_key#11){--..}, at: [<ffffffff802d2529>] configfs_detach_prep+0x54/0x89 + +but task is already holding lock: + (&sb->s_type->i_mutex_key#11){--..}, at: [<ffffffff80296513>] vfs_rmdir+0x49/0xac + +other info that might help us debug this: +2 locks held by rmdir/1499: + #0: (&sb->s_type->i_mutex_key#3/1){--..}, at: [<ffffffff80298138>] do_rmdir+0x82/0x108 + #1: (&sb->s_type->i_mutex_key#11){--..}, at: [<ffffffff80296513>] vfs_rmdir+0x49/0xac + +stack backtrace: +Pid: 1499, comm: rmdir Not tainted 2.6.26-rc3 #5 + +Call Trace: + [<ffffffff8024afe9>] __lock_acquire+0x8d2/0xc78 + [<ffffffff80266399>] filemap_fault+0x1cf/0x332 + [<ffffffff802d2529>] configfs_detach_prep+0x54/0x89 + [<ffffffff8024b762>] lock_acquire+0x51/0x6c + [<ffffffff802d2529>] configfs_detach_prep+0x54/0x89 + [<ffffffff80248331>] debug_mutex_lock_common+0x16/0x23 + [<ffffffff805d6244>] mutex_lock_nested+0xcd/0x23b + [<ffffffff802d2529>] configfs_detach_prep+0x54/0x89 + [<ffffffff802d3656>] configfs_rmdir+0xb8/0x1c3 + [<ffffffff80296535>] vfs_rmdir+0x6b/0xac + [<ffffffff8029816d>] do_rmdir+0xb7/0x108 + [<ffffffff8024a2a2>] trace_hardirqs_on+0xef/0x113 + [<ffffffff805d7364>] trace_hardirqs_on_thunk+0x35/0x3a + [<ffffffff8020b0cb>] system_call_after_swapgs+0x7b/0x80 + + +Signed-off-by: Louis Rilling <Louis.Rilling@kerlabs.com> +--- + fs/configfs/dir.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++------ + 1 file changed, 66 insertions(+), 8 deletions(-) + +Index: b/fs/configfs/dir.c +=================================================================== +--- a/fs/configfs/dir.c 2008-05-22 13:36:06.000000000 +0200 ++++ b/fs/configfs/dir.c 2008-05-23 16:46:43.000000000 +0200 +@@ -37,16 +37,32 @@ + DECLARE_RWSEM(configfs_rename_sem); + + #ifdef CONFIG_LOCKDEP ++static inline int __future_dirent_lock_level(struct configfs_dirent *prev_sd, ++ struct configfs_dirent *sd) ++{ ++ int lock_level = prev_sd->s_lock_level + 1; ++ return (lock_level + I_MUTEX_CHILD < MAX_LOCKDEP_SUBCLASSES) ? ++ lock_level : -ELOOP; ++} ++ ++static inline void __set_dirent_lock_level(struct configfs_dirent *sd, ++ int lock_level) ++{ ++ sd->s_lock_level = lock_level; ++} ++ + static inline int set_dirent_lock_level(struct configfs_dirent *prev_sd, + struct configfs_dirent *sd) + { +- int lock_level = prev_sd->s_lock_level + 1; +- if (lock_level + I_MUTEX_CHILD < MAX_LOCKDEP_SUBCLASSES) { +- sd->s_lock_level = lock_level; +- return lock_level; +- } +- sd->s_lock_level = -1; +- return -ELOOP; ++ int lock_level = __future_dirent_lock_level(prev_sd, sd); ++ __set_dirent_lock_level(sd, lock_level < 0 ? -1 : lock_level); ++ return lock_level; ++} ++ ++static inline void copy_dirent_lock_level(struct configfs_dirent *from, ++ struct configfs_dirent *to) ++{ ++ to->s_lock_level = from->s_lock_level; + } + + static inline void reset_dirent_lock_level(struct configfs_dirent *sd) +@@ -56,12 +72,28 @@ static inline void reset_dirent_lock_lev + + #else /* CONFIG_LOCKDEP */ + ++static inline int __future_dirent_lock_level(struct configfs_dirent *prev_sd, ++ struct configfs_dirent *sd) ++{ ++ return 0; ++} ++ ++static inline void __set_dirent_lock_level(struct configfs_dirent *sd, ++ int lock_level) ++{ ++} ++ + static inline int set_dirent_lock_level(struct configfs_dirent *prev_sd, + struct configfs_dirent *sd) + { + return 0; + } + ++static inline void copy_dirent_lock_level(struct configfs_dirent *from, ++ struct configfs_dirent *to) ++{ ++} ++ + static inline void reset_dirent_lock_level(struct configfs_dirent *sd) + { + } +@@ -372,6 +404,7 @@ static int configfs_detach_prep(struct d + { + struct configfs_dirent *parent_sd = dentry->d_fsdata; + struct configfs_dirent *sd; ++ int lock_level; + int ret; + + ret = -EBUSY; +@@ -383,7 +416,19 @@ static int configfs_detach_prep(struct d + if (sd->s_type & CONFIGFS_NOT_PINNED) + continue; + if (sd->s_type & CONFIGFS_USET_DEFAULT) { +- mutex_lock(&sd->s_dentry->d_inode->i_mutex); ++ /* Do not set lock_level before we acquire the mutex, ++ * otherwise a racing mkdir in sd could start from a ++ * too high lock_level */ ++ lock_level = __future_dirent_lock_level(parent_sd, sd); ++ if (lock_level < 0) { ++ /* Bad bad bad! We cannot remove a directory ++ * that we let be created! */ ++ ret = -ELOOP; ++ break; ++ } ++ mutex_lock_nested(&sd->s_dentry->d_inode->i_mutex, ++ I_MUTEX_CHILD + lock_level); ++ __set_dirent_lock_level(sd, lock_level); + /* Mark that we've taken i_mutex */ + sd->s_type |= CONFIGFS_USET_DROPPING; + +@@ -392,6 +437,10 @@ static int configfs_detach_prep(struct d + * deep nesting of default_groups + */ + ret = configfs_detach_prep(sd->s_dentry); ++ /* Update parent's lock_level so that remaining ++ * sibling children keep on globally increasing ++ * lock_level */ ++ copy_dirent_lock_level(sd, parent_sd); + if (!ret) + continue; + } else +@@ -419,6 +468,7 @@ static void configfs_detach_rollback(str + + if (sd->s_type & CONFIGFS_USET_DROPPING) { + sd->s_type &= ~CONFIGFS_USET_DROPPING; ++ reset_dirent_lock_level(sd); + mutex_unlock(&sd->s_dentry->d_inode->i_mutex); + } + } +@@ -1206,9 +1256,14 @@ static int configfs_rmdir(struct inode * + return -EINVAL; + } + ++ /* The inode of the config_item being removed is already locked by ++ * vfs_rmdir() with subclass I_MUTEX_CHILD. Account for it. */ ++ set_dirent_lock_level(parent_item->ci_dentry->d_fsdata, sd); + ret = configfs_detach_prep(dentry); ++ reset_dirent_lock_level(sd); + if (ret) { + configfs_detach_rollback(dentry); ++ reset_dirent_lock_level(sd); + config_item_put(parent_item); + return ret; + } +@@ -1492,10 +1547,13 @@ void configfs_unregister_subsystem(struc + + mutex_lock_nested(&configfs_sb->s_root->d_inode->i_mutex, + I_MUTEX_PARENT); ++ /* Sets subsys's configfs_dirent lock_level to 0 */ ++ set_dirent_lock_level(configfs_sb->s_root->d_fsdata, dentry->d_fsdata); + 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"); + } ++ reset_dirent_lock_level(dentry->d_fsdata); + configfs_detach_group(&group->cg_item); + dentry->d_inode->i_flags |= S_DEAD; + mutex_unlock(&dentry->d_inode->i_mutex); diff --git a/a/content_digest b/N1/content_digest index 4c7d282..5b81700 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -1,14 +1,14 @@ "ref\020080522114048.265996107@kerlabs.com\0" "ref\020080522114947.927196541@kerlabs.com\0" "From\0Louis Rilling <Louis.Rilling@kerlabs.com>\0" - "Subject\0[Ocfs2-devel] [RFC][PATCH 4/4] configfs: Make multiple default_group destructions lockdep friendly\0" + "Subject\0Re: [RFC][PATCH 4/4] configfs: Make multiple default_group destructions lockdep friendly\0" "Date\0Fri, 23 May 2008 18:44:58 +0200\0" "To\0Joel.Becker@oracle.com\0" "Cc\0ocfs2-devel@oss.oracle.com" " linux-kernel@vger.kernel.org\0" - "\00:1\0" + "\01:1\0" "b\0" - "Louis Rilling a ?crit :\n" + "Louis Rilling a \303\251crit :\n" "> When destroying a config_group having multiple (nested or not) default groups,\n" "> lockdep raises a warning because multiple locks of class I_MUTEX_NORMAL are\n" "> taken in configfs_detach_prep().\n" @@ -73,13 +73,214 @@ "Dr Louis Rilling\t\t\tKerlabs\n" "Skype: louis.rilling\t\t\tBatiment Germanium\n" "Phone: (+33|0) 6 80 89 08 23\t\t80 avenue des Buttes de Coesmes\n" - "http://www.kerlabs.com/\t\t\t35700 Rennes\n" - "-------------- next part --------------\n" - "A non-text attachment was scrubbed...\n" - "Name: configfs-make-default-group-destruction-lockdep-friendly-2.patch\n" - "Type: text/x-diff\n" - "Size: 7137 bytes\n" - "Desc: not available\n" - Url : http://oss.oracle.com/pipermail/ocfs2-devel/attachments/20080523/53738fb6/attachment.bin + "http://www.kerlabs.com/\t\t\t35700 Rennes" + "\01:2\0" + "fn\0configfs-make-default-group-destruction-lockdep-friendly-2.patch\0" + "b\0" + "configfs: Make multiple default_group destructions lockdep friendly\n" + "\n" + "When destroying a config_group having multiple (nested or not) default groups,\n" + "lockdep raises a warning because multiple locks of class I_MUTEX_NORMAL are\n" + "taken in configfs_detach_prep().\n" + "\n" + "This patch makes such default group destructions lockdep-friendly by increasing\n" + "the i_mutex sub-class from I_MUTEX_CHILD + 1 onwards for each default group\n" + "locked under a being-destroyed config_group.\n" + "\n" + "With this patch and lockdep compiled-in, the number of default groups (whatever\n" + "their depth in the tree) under a user-created config_group (resp. registered\n" + "subsystem) is limited to MAX_LOCKDEP_SUBCLASSES - I_MUTEX_CHILD - 1 == 3. This\n" + "limit is removed when not compiling lockdep.\n" + "\n" + "=============================================\n" + "[ INFO: possible recursive locking detected ]\n" + "2.6.26-rc3 #5\n" + "---------------------------------------------\n" + "rmdir/1499 is trying to acquire lock:\n" + " (&sb->s_type->i_mutex_key#11){--..}, at: [<ffffffff802d2529>] configfs_detach_prep+0x54/0x89\n" + "\n" + "but task is already holding lock:\n" + " (&sb->s_type->i_mutex_key#11){--..}, at: [<ffffffff80296513>] vfs_rmdir+0x49/0xac\n" + "\n" + "other info that might help us debug this:\n" + "2 locks held by rmdir/1499:\n" + " #0: (&sb->s_type->i_mutex_key#3/1){--..}, at: [<ffffffff80298138>] do_rmdir+0x82/0x108\n" + " #1: (&sb->s_type->i_mutex_key#11){--..}, at: [<ffffffff80296513>] vfs_rmdir+0x49/0xac\n" + "\n" + "stack backtrace:\n" + "Pid: 1499, comm: rmdir Not tainted 2.6.26-rc3 #5\n" + "\n" + "Call Trace:\n" + " [<ffffffff8024afe9>] __lock_acquire+0x8d2/0xc78\n" + " [<ffffffff80266399>] filemap_fault+0x1cf/0x332\n" + " [<ffffffff802d2529>] configfs_detach_prep+0x54/0x89\n" + " [<ffffffff8024b762>] lock_acquire+0x51/0x6c\n" + " [<ffffffff802d2529>] configfs_detach_prep+0x54/0x89\n" + " [<ffffffff80248331>] debug_mutex_lock_common+0x16/0x23\n" + " [<ffffffff805d6244>] mutex_lock_nested+0xcd/0x23b\n" + " [<ffffffff802d2529>] configfs_detach_prep+0x54/0x89\n" + " [<ffffffff802d3656>] configfs_rmdir+0xb8/0x1c3\n" + " [<ffffffff80296535>] vfs_rmdir+0x6b/0xac\n" + " [<ffffffff8029816d>] do_rmdir+0xb7/0x108\n" + " [<ffffffff8024a2a2>] trace_hardirqs_on+0xef/0x113\n" + " [<ffffffff805d7364>] trace_hardirqs_on_thunk+0x35/0x3a\n" + " [<ffffffff8020b0cb>] system_call_after_swapgs+0x7b/0x80\n" + "\n" + "\n" + "Signed-off-by: Louis Rilling <Louis.Rilling@kerlabs.com>\n" + "---\n" + " fs/configfs/dir.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++------\n" + " 1 file changed, 66 insertions(+), 8 deletions(-)\n" + "\n" + "Index: b/fs/configfs/dir.c\n" + "===================================================================\n" + "--- a/fs/configfs/dir.c\t2008-05-22 13:36:06.000000000 +0200\n" + "+++ b/fs/configfs/dir.c\t2008-05-23 16:46:43.000000000 +0200\n" + "@@ -37,16 +37,32 @@\n" + " DECLARE_RWSEM(configfs_rename_sem);\n" + " \n" + " #ifdef CONFIG_LOCKDEP\n" + "+static inline int __future_dirent_lock_level(struct configfs_dirent *prev_sd,\n" + "+\t\t\t\t\t struct configfs_dirent *sd)\n" + "+{\n" + "+\tint lock_level = prev_sd->s_lock_level + 1;\n" + "+\treturn (lock_level + I_MUTEX_CHILD < MAX_LOCKDEP_SUBCLASSES) ?\n" + "+\t\tlock_level : -ELOOP;\n" + "+}\n" + "+\n" + "+static inline void __set_dirent_lock_level(struct configfs_dirent *sd,\n" + "+\t\t\t\t\t int lock_level)\n" + "+{\n" + "+\tsd->s_lock_level = lock_level;\n" + "+}\n" + "+\n" + " static inline int set_dirent_lock_level(struct configfs_dirent *prev_sd,\n" + " \t\t\t\t\tstruct configfs_dirent *sd)\n" + " {\n" + "-\tint lock_level = prev_sd->s_lock_level + 1;\n" + "-\tif (lock_level + I_MUTEX_CHILD < MAX_LOCKDEP_SUBCLASSES) {\n" + "-\t\tsd->s_lock_level = lock_level;\n" + "-\t\treturn lock_level;\n" + "-\t}\n" + "-\tsd->s_lock_level = -1;\n" + "-\treturn -ELOOP;\n" + "+\tint lock_level = __future_dirent_lock_level(prev_sd, sd);\n" + "+\t__set_dirent_lock_level(sd, lock_level < 0 ? -1 : lock_level);\n" + "+\treturn lock_level;\n" + "+}\n" + "+\n" + "+static inline void copy_dirent_lock_level(struct configfs_dirent *from,\n" + "+\t\t\t\t\t struct configfs_dirent *to)\n" + "+{\n" + "+\tto->s_lock_level = from->s_lock_level;\n" + " }\n" + " \n" + " static inline void reset_dirent_lock_level(struct configfs_dirent *sd)\n" + "@@ -56,12 +72,28 @@ static inline void reset_dirent_lock_lev\n" + " \n" + " #else /* CONFIG_LOCKDEP */\n" + " \n" + "+static inline int __future_dirent_lock_level(struct configfs_dirent *prev_sd,\n" + "+\t\t\t\t\t struct configfs_dirent *sd)\n" + "+{\n" + "+\treturn 0;\n" + "+}\n" + "+\n" + "+static inline void __set_dirent_lock_level(struct configfs_dirent *sd,\n" + "+\t\t\t\t\t int lock_level)\n" + "+{\n" + "+}\n" + "+\n" + " static inline int set_dirent_lock_level(struct configfs_dirent *prev_sd,\n" + " \t\t\t\t\tstruct configfs_dirent *sd)\n" + " {\n" + " \treturn 0;\n" + " }\n" + " \n" + "+static inline void copy_dirent_lock_level(struct configfs_dirent *from,\n" + "+\t\t\t\t\t struct configfs_dirent *to)\n" + "+{\n" + "+}\n" + "+\n" + " static inline void reset_dirent_lock_level(struct configfs_dirent *sd)\n" + " {\n" + " }\n" + "@@ -372,6 +404,7 @@ static int configfs_detach_prep(struct d\n" + " {\n" + " \tstruct configfs_dirent *parent_sd = dentry->d_fsdata;\n" + " \tstruct configfs_dirent *sd;\n" + "+\tint lock_level;\n" + " \tint ret;\n" + " \n" + " \tret = -EBUSY;\n" + "@@ -383,7 +416,19 @@ static int configfs_detach_prep(struct d\n" + " \t\tif (sd->s_type & CONFIGFS_NOT_PINNED)\n" + " \t\t\tcontinue;\n" + " \t\tif (sd->s_type & CONFIGFS_USET_DEFAULT) {\n" + "-\t\t\tmutex_lock(&sd->s_dentry->d_inode->i_mutex);\n" + "+\t\t\t/* Do not set lock_level before we acquire the mutex,\n" + "+\t\t\t * otherwise a racing mkdir in sd could start from a\n" + "+\t\t\t * too high lock_level */\n" + "+\t\t\tlock_level = __future_dirent_lock_level(parent_sd, sd);\n" + "+\t\t\tif (lock_level < 0) {\n" + "+\t\t\t\t/* Bad bad bad! We cannot remove a directory\n" + "+\t\t\t\t * that we let be created! */\n" + "+\t\t\t\tret = -ELOOP;\n" + "+\t\t\t\tbreak;\n" + "+\t\t\t}\n" + "+\t\t\tmutex_lock_nested(&sd->s_dentry->d_inode->i_mutex,\n" + "+\t\t\t\t\t I_MUTEX_CHILD + lock_level);\n" + "+\t\t\t__set_dirent_lock_level(sd, lock_level);\n" + " \t\t\t/* Mark that we've taken i_mutex */\n" + " \t\t\tsd->s_type |= CONFIGFS_USET_DROPPING;\n" + " \n" + "@@ -392,6 +437,10 @@ static int configfs_detach_prep(struct d\n" + " \t\t\t * deep nesting of default_groups\n" + " \t\t\t */\n" + " \t\t\tret = configfs_detach_prep(sd->s_dentry);\n" + "+\t\t\t/* Update parent's lock_level so that remaining\n" + "+\t\t\t * sibling children keep on globally increasing\n" + "+\t\t\t * lock_level */\n" + "+\t\t\tcopy_dirent_lock_level(sd, parent_sd);\n" + " \t\t\tif (!ret)\n" + " \t\t\t\tcontinue;\n" + " \t\t} else\n" + "@@ -419,6 +468,7 @@ static void configfs_detach_rollback(str\n" + " \n" + " \t\t\tif (sd->s_type & CONFIGFS_USET_DROPPING) {\n" + " \t\t\t\tsd->s_type &= ~CONFIGFS_USET_DROPPING;\n" + "+\t\t\t\treset_dirent_lock_level(sd);\n" + " \t\t\t\tmutex_unlock(&sd->s_dentry->d_inode->i_mutex);\n" + " \t\t\t}\n" + " \t\t}\n" + "@@ -1206,9 +1256,14 @@ static int configfs_rmdir(struct inode *\n" + " \t\treturn -EINVAL;\n" + " \t}\n" + " \n" + "+\t/* The inode of the config_item being removed is already locked by\n" + "+\t * vfs_rmdir() with subclass I_MUTEX_CHILD. Account for it. */\n" + "+\tset_dirent_lock_level(parent_item->ci_dentry->d_fsdata, sd);\n" + " \tret = configfs_detach_prep(dentry);\n" + "+\treset_dirent_lock_level(sd);\n" + " \tif (ret) {\n" + " \t\tconfigfs_detach_rollback(dentry);\n" + "+\t\treset_dirent_lock_level(sd);\n" + " \t\tconfig_item_put(parent_item);\n" + " \t\treturn ret;\n" + " \t}\n" + "@@ -1492,10 +1547,13 @@ void configfs_unregister_subsystem(struc\n" + " \n" + " \tmutex_lock_nested(&configfs_sb->s_root->d_inode->i_mutex,\n" + " \t\t\t I_MUTEX_PARENT);\n" + "+\t/* Sets subsys's configfs_dirent lock_level to 0 */\n" + "+\tset_dirent_lock_level(configfs_sb->s_root->d_fsdata, dentry->d_fsdata);\n" + " \tmutex_lock_nested(&dentry->d_inode->i_mutex, I_MUTEX_CHILD);\n" + " \tif (configfs_detach_prep(dentry)) {\n" + " \t\tprintk(KERN_ERR \"configfs: Tried to unregister non-empty subsystem!\\n\");\n" + " \t}\n" + "+\treset_dirent_lock_level(dentry->d_fsdata);\n" + " \tconfigfs_detach_group(&group->cg_item);\n" + " \tdentry->d_inode->i_flags |= S_DEAD;\n" + " \tmutex_unlock(&dentry->d_inode->i_mutex);" -ff689bba34ff299de2691309c1987373ecf3c837c7647b37d65dbe747f8c6a7b +b62717ab09c0cb598142f1720b6c501155f9a5f579361180621de96bd2e4f99a
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.