Re: [Patch] cpusets: fix race in cpuset_add_file()

[Paul Jackson <pj@sgi.com>]

You can continue to ignore this patch, Andrew.  I'm still thinking it
through with Simon.

Here's another possible way to skin this cat, Simon.

Instead of adding an inode lock, how about just extending the cpuset_sem
window.  If we hold cpuset_sem for the entire cpuset_mkdir() operation,
then no other cpuset_mkdir can overlap, and there should be no
confused overlapping directory creations.

This reduces the risks of unrecognized A-B-A deadlocks, and it removes
the concern I have that dropping the cpuset_sem before we're done opens
the way for more inconsistencies.

This needs to be tested before it goes in - there is a non-zero risk
that I made a stupid mistake and it locks up or something.

Signed-off-by: Paul Jackson <pj@sgi.com>

Index: 2.6.9-rc2-mm1/kernel/cpuset.c
===================================================================
--- 2.6.9-rc2-mm1.orig/kernel/cpuset.c	2004-09-16 23:46:01.000000000 -0700
+++ 2.6.9-rc2-mm1/kernel/cpuset.c	2004-09-17 00:19:02.000000000 -0700
@@ -1235,7 +1235,6 @@ static long cpuset_create(struct cpuset 
 	if (!cs)
 		return -ENOMEM;
 
-	down(&cpuset_sem);
 	cs->flags = 0;
 	if (notify_on_release(parent))
 		set_bit(CS_NOTIFY_ON_RELEASE, &cs->flags);
@@ -1256,22 +1255,23 @@ static long cpuset_create(struct cpuset 
 		goto err;
 	err = cpuset_populate_dir(cs->dentry);
 	/* If err < 0, we have a half-filled directory - oh well ;) */
-	up(&cpuset_sem);
 	return 0;
 err:
 	list_del(&cs->sibling);
-	up(&cpuset_sem);
 	kfree(cs);
 	return err;
 }
 
 static int cpuset_mkdir(struct inode *dir, struct dentry *dentry, int mode)
 {
-	struct dentry *d_parent = dentry->d_parent;
-	struct cpuset *c_parent = (struct cpuset *)d_parent->d_fsdata;
+	struct cpuset *c_parent;
+	int rc;
 
-	/* the vfs holds inode->i_sem already */
-	return cpuset_create(c_parent, dentry->d_name.name, mode | S_IFDIR);
+	down(&cpuset_sem);
+	c_parent = dentry->d_parent->d_fsdata;
+	rc = cpuset_create(c_parent, dentry->d_name.name, mode | S_IFDIR);
+	up(&cpuset_sem);
+	return rc;
 }
 
 static int cpuset_rmdir(struct inode *unused_dir, struct dentry *dentry)


-- 
                          I won't rest till it's the best ...
                          Programmer, Linux Scalability
                          Paul Jackson <pj@sgi.com> 1.650.933.1373