[patch] cgroup fs: avoid switching ->d_op on live dentry

* [patch] cgroup fs: avoid switching ->d_op on live dentry
@ 2010-12-21  7:12 Nick Piggin
  2010-12-21  9:04 ` Paul Menage
  0 siblings, 1 reply; 13+ messages in thread
From: Nick Piggin @ 2010-12-21  7:12 UTC (permalink / raw)
  To: Paul Menage, Li Zefan, linux-fsdevel


cgroup fs: avoid switching ->d_op on live dentry

Switching d_op on a live dentry is racy in general, so avoid it. In this case
it is a negative dentry, which is safer, but there are still concurrent ops
which may be called on d_op in that case (eg. d_revalidate). So in general
a filesystem may not do this. Fix cgroupfs so as not to do this.

This caused problematic interaction with other development work, thanks
to Sedat Dilek and Eric Paris for reporting the issue here:

  http://marc.info/?l=linux-next&m=129287924727475&w=2

Signed-off-by: Nick Piggin <npiggin@kernel.dk>

---
One of the patches in my vfs scaling series tripped over this, comments?

 kernel/cgroup.c |   27 ++++++++++++++++++++++-----
 1 file changed, 22 insertions(+), 5 deletions(-)

Index: linux-2.6/kernel/cgroup.c
===================================================================

--- linux-2.6.orig/kernel/cgroup.c	2010-12-21 16:02:32.000000000 +1100
+++ linux-2.6/kernel/cgroup.c	2010-12-21 16:51:39.000000000 +1100
@@ -763,6 +763,8 @@ EXPORT_SYMBOL_GPL(cgroup_unlock);
  * -> cgroup_mkdir.
  */
 
+static struct dentry *cgroup_lookup(struct inode *dir,
+			struct dentry *dentry, struct nameidata *nd);
 static int cgroup_mkdir(struct inode *dir, struct dentry *dentry, int mode);
 static int cgroup_rmdir(struct inode *unused_dir, struct dentry *dentry);
 static int cgroup_populate_dir(struct cgroup *cgrp);
@@ -2180,7 +2182,7 @@ static const struct file_operations cgro
 };
 
 static const struct inode_operations cgroup_dir_inode_operations = {
-	.lookup = simple_lookup,
+	.lookup = cgroup_lookup,
 	.mkdir = cgroup_mkdir,
 	.rmdir = cgroup_rmdir,
 	.rename = cgroup_rename,
@@ -2196,13 +2198,29 @@ static inline struct cftype *__file_cft(
 	return __d_cft(file->f_dentry);
 }
 
-static int cgroup_create_file(struct dentry *dentry, mode_t mode,
-				struct super_block *sb)
+static int cgroup_delete_dentry(struct dentry *dentry)
+{
+	return 1;
+}
+
+static struct dentry *cgroup_lookup(struct inode *dir,
+			struct dentry *dentry, struct nameidata *nd)
 {
-	static const struct dentry_operations cgroup_dops = {
+	static const struct dentry_operations cgroup_dentry_operations = {
+		.d_delete = cgroup_delete_dentry,
 		.d_iput = cgroup_diput,
 	};
 
+	if (dentry->d_name.len > NAME_MAX)
+		return ERR_PTR(-ENAMETOOLONG);
+	dentry->d_op = &cgroup_dentry_operations;
+	d_add(dentry, NULL);
+	return NULL;
+}
+
+static int cgroup_create_file(struct dentry *dentry, mode_t mode,
+				struct super_block *sb)
+{
 	struct inode *inode;
 
 	if (!dentry)
@@ -2228,7 +2246,6 @@ static int cgroup_create_file(struct den
 		inode->i_size = 0;
 		inode->i_fop = &cgroup_file_operations;
 	}
-	dentry->d_op = &cgroup_dops;
 	d_instantiate(dentry, inode);
 	dget(dentry);	/* Extra count - pin the dentry in core */
 	return 0;

^ permalink raw reply	[flat|nested] 13+ messages in thread