linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [patch] cgroup fs: avoid switching ->d_op on live dentry
@ 2010-12-21  8:44 Sedat Dilek
  2010-12-21  8:55 ` Nick Piggin
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Sedat Dilek @ 2010-12-21  8:44 UTC (permalink / raw)
  To: Nick Piggin; +Cc: systemd-devel, linux-fsdevel, linux-next, eparis

Against linux-next (next-20101210) it should look like:

$ diff -Naur cgroup-fix/cgroup-fs-avoid-switching-d_op-on-live-dentry.patch
cgroup-fix/cgroup-fs-avoid-switching-d_op-on-live-dentry-v2.patch
--- cgroup-fix/cgroup-fs-avoid-switching-d_op-on-live-dentry.patch
 2010-12-21 09:31:38.649601964 +0100
+++ cgroup-fix/cgroup-fs-avoid-switching-d_op-on-live-dentry-v2.patch
 2010-12-21 09:40:21.151033232 +0100
@@ -83,7 +83,7 @@
                inode->i_size = 0;
                inode->i_fop = &cgroup_file_operations;
        }
--      dentry->d_op = &cgroup_dops;
+-      d_set_d_op(dentry, &cgroup_dops);
        d_instantiate(dentry, inode);
        dget(dentry);   /* Extra count - pin the dentry in core */
        return 0;

- Sedat -

^ permalink raw reply	[flat|nested] 13+ messages in thread
* [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
end of thread, other threads:[~2010-12-22  4:38 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-12-21  8:44 [patch] cgroup fs: avoid switching ->d_op on live dentry Sedat Dilek
2010-12-21  8:55 ` Nick Piggin
2010-12-21 13:35   ` Sedat Dilek
2010-12-21 18:25   ` Al Viro
2010-12-21 18:57     ` Sedat Dilek
2010-12-22  3:11     ` Nick Piggin
2010-12-22  3:31       ` Sedat Dilek
2010-12-22  4:38     ` Sedat Dilek
2010-12-21 15:12 ` Sedat Dilek
2010-12-21 15:28 ` Sedat Dilek
2010-12-21 17:56   ` Sedat Dilek
  -- strict thread matches above, loose matches on Subject: below --
2010-12-21  7:12 Nick Piggin
2010-12-21  9:04 ` Paul Menage

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).