aboutsummaryrefslogtreecommitdiffstats
path: root/fs/configfs/inode.c
diff options
context:
space:
mode:
authorLouis Rilling <louis.rilling@kerlabs.com>2009-01-28 13:18:32 -0500
committerJoel Becker <joel.becker@oracle.com>2009-04-30 13:48:23 -0400
commite74cc06df3b05e2b2c1611a043f6e6dcadaab1eb (patch)
treec514c5f5149ce26ef9b44473bed18a7836540a38 /fs/configfs/inode.c
parent3c48f23adada870db612a0dd3488605c4af5c0a5 (diff)
configfs: Silence lockdep on mkdir() and rmdir()
When attaching default groups (subdirs) of a new group (in mkdir() or in configfs_register()), configfs recursively takes inode's mutexes along the path from the parent of the new group to the default subdirs. This is needed to ensure that the VFS will not race with operations on these sub-dirs. This is safe for the following reasons: - the VFS allows one to lock first an inode and second one of its children (The lock subclasses for this pattern are respectively I_MUTEX_PARENT and I_MUTEX_CHILD); - from this rule any inode path can be recursively locked in descending order as long as it stays under a single mountpoint and does not follow symlinks. Unfortunately lockdep does not know (yet?) how to handle such recursion. I've tried to use Peter Zijlstra's lock_set_subclass() helper to upgrade i_mutexes from I_MUTEX_CHILD to I_MUTEX_PARENT when we know that we might recursively lock some of their descendant, but this usage does not seem to fit the purpose of lock_set_subclass() because it leads to several i_mutex locked with subclass I_MUTEX_PARENT by the same task. >From inside configfs it is not possible to serialize those recursive locking with a top-level one, because mkdir() and rmdir() are already called with inodes locked by the VFS. So using some mutex_lock_nest_lock() is not an option. I am proposing two solutions: 1) one that wraps recursive mutex_lock()s with lockdep_off()/lockdep_on(). 2) (as suggested earlier by Peter Zijlstra) one that puts the i_mutexes recursively locked in different classes based on their depth from the top-level config_group created. This induces an arbitrary limit (MAX_LOCK_DEPTH - 2 == 46) on the nesting of configfs default groups whenever lockdep is activated but this limit looks reasonably high. Unfortunately, this also isolates VFS operations on configfs default groups from the others and thus lowers the chances to detect locking issues. Nobody likes solution 1), which I can understand. This patch implements solution 2). However lockdep is still not happy with configfs_depend_item(). Next patch reworks the locking of configfs_depend_item() and finally makes lockdep happy. [ Note: This hides a few locking interactions with the VFS from lockdep. That was my big concern, because we like lockdep's protection. However, the current state always dumps a spurious warning. The locking is correct, so I tell people to ignore the warning and that we'll keep our eyes on the locking to make sure it stays correct. With this patch, we eliminate the warning. We do lose some of the lockdep protections, but this only means that we still have to keep our eyes on the locking. We're going to do that anyway. -- Joel ] Signed-off-by: Louis Rilling <louis.rilling@kerlabs.com> Signed-off-by: Joel Becker <joel.becker@oracle.com>
Diffstat (limited to 'fs/configfs/inode.c')
-rw-r--r--fs/configfs/inode.c38
1 files changed, 38 insertions, 0 deletions
diff --git a/fs/configfs/inode.c b/fs/configfs/inode.c
index 5d349d38e056..4921e7426d95 100644
--- a/fs/configfs/inode.c
+++ b/fs/configfs/inode.c
@@ -33,10 +33,15 @@
33#include <linux/backing-dev.h> 33#include <linux/backing-dev.h>
34#include <linux/capability.h> 34#include <linux/capability.h>
35#include <linux/sched.h> 35#include <linux/sched.h>
36#include <linux/lockdep.h>
36 37
37#include <linux/configfs.h> 38#include <linux/configfs.h>
38#include "configfs_internal.h" 39#include "configfs_internal.h"
39 40
41#ifdef CONFIG_LOCKDEP
42static struct lock_class_key default_group_class[MAX_LOCK_DEPTH];
43#endif
44
40extern struct super_block * configfs_sb; 45extern struct super_block * configfs_sb;
41 46
42static const struct address_space_operations configfs_aops = { 47static const struct address_space_operations configfs_aops = {
@@ -150,6 +155,38 @@ struct inode * configfs_new_inode(mode_t mode, struct configfs_dirent * sd)
150 return inode; 155 return inode;
151} 156}
152 157
158#ifdef CONFIG_LOCKDEP
159
160static void configfs_set_inode_lock_class(struct configfs_dirent *sd,
161 struct inode *inode)
162{
163 int depth = sd->s_depth;
164
165 if (depth > 0) {
166 if (depth <= ARRAY_SIZE(default_group_class)) {
167 lockdep_set_class(&inode->i_mutex,
168 &default_group_class[depth - 1]);
169 } else {
170 /*
171 * In practice the maximum level of locking depth is
172 * already reached. Just inform about possible reasons.
173 */
174 printk(KERN_INFO "configfs: Too many levels of inodes"
175 " for the locking correctness validator.\n");
176 printk(KERN_INFO "Spurious warnings may appear.\n");
177 }
178 }
179}
180
181#else /* CONFIG_LOCKDEP */
182
183static void configfs_set_inode_lock_class(struct configfs_dirent *sd,
184 struct inode *inode)
185{
186}
187
188#endif /* CONFIG_LOCKDEP */
189
153int configfs_create(struct dentry * dentry, int mode, int (*init)(struct inode *)) 190int configfs_create(struct dentry * dentry, int mode, int (*init)(struct inode *))
154{ 191{
155 int error = 0; 192 int error = 0;
@@ -162,6 +199,7 @@ int configfs_create(struct dentry * dentry, int mode, int (*init)(struct inode *
162 struct inode *p_inode = dentry->d_parent->d_inode; 199 struct inode *p_inode = dentry->d_parent->d_inode;
163 p_inode->i_mtime = p_inode->i_ctime = CURRENT_TIME; 200 p_inode->i_mtime = p_inode->i_ctime = CURRENT_TIME;
164 } 201 }
202 configfs_set_inode_lock_class(sd, inode);
165 goto Proceed; 203 goto Proceed;
166 } 204 }
167 else 205 else