[patch 12/40] reiserfs: simplify xattr internal file lookups/opens

[jeffm@suse.com]

[-- Attachment #1: reiserfs-simplify-xattr-internal-file-lookups-opens.diff --]
[-- Type: text/plain, Size: 10370 bytes --]

 The xattr file open/lookup code is needlessly complex. We can use vfs-level
 operations to perform the same work, and also simplify the locking
 constraints. The locking advantages will be exploited in future patches.

Signed-off-by: Jeff Mahoney <jeffm@suse.com>

---
 fs/reiserfs/xattr.c |  200 +++++++++++++++++-----------------------------------
 1 file changed, 66 insertions(+), 134 deletions(-)

--- a/fs/reiserfs/xattr.c	2007-06-11 14:49:35.000000000 -0400
+++ b/fs/reiserfs/xattr.c	2007-06-11 14:50:08.000000000 -0400
@@ -46,173 +46,104 @@
 #include <linux/stat.h>
 #include <asm/semaphore.h>
 
-#define FL_READONLY 128
-#define FL_DIR_SEM_HELD 256
 #define PRIVROOT_NAME ".reiserfs_priv"
 #define XAROOT_NAME   "xattrs"
 
 static struct reiserfs_xattr_handler *find_xattr_handler_prefix(const char
 								*prefix);
 
-/* Returns the dentry referring to the root of the extended attribute
- * directory tree. If it has already been retrieved, it is used. If it
- * hasn't been created and the flags indicate creation is allowed, we
- * attempt to create it. On error, we return a pointer-encoded error.
- */
-static struct dentry *get_xa_root(struct super_block *sb, int flags)
-{
-	struct dentry *privroot = dget(REISERFS_SB(sb)->priv_root);
-	struct dentry *xaroot;
+#define xattr_may_create(flags)	(!flags || flags & XATTR_CREATE)
 
-	/* This needs to be created at mount-time */
-	if (!privroot)
-		return ERR_PTR(-ENODATA);
+static struct dentry *lookup_or_create_dir(struct dentry *parent,
+                                           const char *name, int flags)
+{
+	struct dentry *dentry;
+	BUG_ON(!parent);
 
-	mutex_lock_nested(&privroot->d_inode->i_mutex, I_MUTEX_XATTR);
-	if (REISERFS_SB(sb)->xattr_root) {
-		xaroot = dget(REISERFS_SB(sb)->xattr_root);
-		goto out;
-	}
+	mutex_lock_nested(&parent->d_inode->i_mutex, I_MUTEX_XATTR);
 
-	xaroot = lookup_one_len(XAROOT_NAME, privroot, strlen(XAROOT_NAME));
-	if (IS_ERR(xaroot)) {
-		goto out;
-	} else if (!xaroot->d_inode) {
+	dentry = lookup_one_len(name, parent, strlen(name));
+	if (IS_ERR(dentry)) {
+		mutex_unlock(&parent->d_inode->i_mutex);
+		return dentry;
+	} else if (!dentry->d_inode) {
 		int err = -ENODATA;
-		if (flags == 0 || flags & XATTR_CREATE)
-			err = privroot->d_inode->i_op->mkdir(privroot->d_inode,
-			                                     xaroot, 0700);
+
+		if (xattr_may_create(flags))
+			err = vfs_mkdir(parent->d_inode, dentry, 0700);
+
 		if (err) {
-			dput(xaroot);
-			xaroot = ERR_PTR(err);
-			goto out;
+			dput(dentry);
+			dentry = ERR_PTR(err);
 		}
 	}
-	REISERFS_SB(sb)->xattr_root = dget(xaroot);
 
-      out:
-	mutex_unlock(&privroot->d_inode->i_mutex);
-	dput(privroot);
-	return xaroot;
+	mutex_unlock(&parent->d_inode->i_mutex);
+	return dentry;
+}
+
+static struct dentry *open_xa_root(struct super_block *sb, int flags)
+{
+	struct dentry *privroot = REISERFS_SB(sb)->priv_root;
+	if (!privroot)
+		return ERR_PTR(-ENODATA);
+	return lookup_or_create_dir(privroot, XAROOT_NAME, flags);
 }
 
-/* Opens the directory corresponding to the inode's extended attribute store.
- * If flags allow, the tree to the directory may be created. If creation is
- * prohibited, -ENODATA is returned. */
 static struct dentry *open_xa_dir(const struct inode *inode, int flags)
 {
 	struct dentry *xaroot, *xadir;
 	char namebuf[17];
 
-	xaroot = get_xa_root(inode->i_sb, flags);
+	xaroot = open_xa_root(inode->i_sb, flags);
 	if (IS_ERR(xaroot))
 		return xaroot;
 
-	/* ok, we have xaroot open */
 	snprintf(namebuf, sizeof(namebuf), "%X.%X",
 		 le32_to_cpu(INODE_PKEY(inode)->k_objectid),
 		 inode->i_generation);
-	xadir = lookup_one_len(namebuf, xaroot, strlen(namebuf));
-	if (IS_ERR(xadir)) {
-		dput(xaroot);
-		return xadir;
-	}
-
-	if (!xadir->d_inode) {
-		int err;
-		if (flags == 0 || flags & XATTR_CREATE) {
-			/* Although there is nothing else trying to create this directory,
-			 * another directory with the same hash may be created, so we need
-			 * to protect against that */
-			err =
-			    xaroot->d_inode->i_op->mkdir(xaroot->d_inode, xadir,
-							 0700);
-			if (err) {
-				dput(xaroot);
-				dput(xadir);
-				return ERR_PTR(err);
-			}
-		}
-		if (!xadir->d_inode) {
-			dput(xaroot);
-			dput(xadir);
-			return ERR_PTR(-ENODATA);
-		}
-	}
 
+	xadir = lookup_or_create_dir(xaroot, namebuf, flags);
 	dput(xaroot);
 	return xadir;
+
 }
 
-/* Returns a dentry corresponding to a specific extended attribute file
- * for the inode. If flags allow, the file is created. Otherwise, a
- * valid or negative dentry, or an error is returned. */
-static struct dentry *get_xa_file_dentry(const struct inode *inode,
-					 const char *name, int flags)
+static struct file *open_xattr_file(const struct inode *inode,
+                                    const char *name, int flags)
 {
 	struct dentry *xadir, *xafile;
 	int err = 0;
 
 	xadir = open_xa_dir(inode, flags);
-	if (IS_ERR(xadir)) {
+	if (IS_ERR(xadir))
 		return ERR_PTR(PTR_ERR(xadir));
-	} else if (xadir && !xadir->d_inode) {
-		dput(xadir);
-		return ERR_PTR(-ENODATA);
-	}
 
+	mutex_lock_nested(&xadir->d_inode->i_mutex, I_MUTEX_XATTR);
 	xafile = lookup_one_len(name, xadir, strlen(name));
 	if (IS_ERR(xafile)) {
-		dput(xadir);
-		return ERR_PTR(PTR_ERR(xafile));
+		err = PTR_ERR(xafile);
+		goto out;
 	}
 
-	if (xafile->d_inode) {	/* file exists */
-		if (flags & XATTR_CREATE) {
-			err = -EEXIST;
-			dput(xafile);
-			goto out;
-		}
-	} else if (flags & XATTR_REPLACE || flags & FL_READONLY) {
-		goto out;
-	} else {
-		/* inode->i_mutex is down, so nothing else can try to create
-		 * the same xattr */
-		err = xadir->d_inode->i_op->create(xadir->d_inode, xafile,
-						   0700 | S_IFREG, NULL);
+	if (xafile->d_inode && (flags & XATTR_CREATE))
+		err = -EEXIST;
 
-		if (err) {
-			dput(xafile);
-			goto out;
-		}
+	if (!xafile->d_inode) {
+		err = -ENODATA;
+		if (xattr_may_create(flags))
+			err = vfs_create(xadir->d_inode, xafile,
+			                 0700|S_IFREG, NULL);
 	}
 
-      out:
-	dput(xadir);
 	if (err)
-		xafile = ERR_PTR(err);
-	return xafile;
-}
-
-/* Opens a file pointer to the attribute associated with inode */
-static struct file *open_xa_file(const struct inode *inode, const char *name,
-				 int flags)
-{
-	struct dentry *xafile;
-	struct file *fp;
-
-	xafile = get_xa_file_dentry(inode, name, flags);
-	if (IS_ERR(xafile))
-		return ERR_PTR(PTR_ERR(xafile));
-	else if (!xafile->d_inode) {
 		dput(xafile);
-		return ERR_PTR(-ENODATA);
-	}
-
-	fp = dentry_open(xafile, NULL, O_RDWR);
-	/* dentry_open dputs the dentry if it fails */
-
-	return fp;
+out:
+	mutex_unlock(&xadir->d_inode->i_mutex);
+	dput(xadir);
+	if (err)
+		return ERR_PTR(err);
+	return dentry_open(xafile, NULL, O_RDWR);
 }
 
 /*
@@ -455,7 +386,7 @@ reiserfs_xattr_set(struct inode *inode, 
 		xahash = xattr_hash(buffer, buffer_size);
 
       open_file:
-	fp = open_xa_file(inode, name, flags);
+	fp = open_xattr_file(inode, name, flags);
 	if (IS_ERR(fp)) {
 		err = PTR_ERR(fp);
 		goto out;
@@ -573,7 +504,7 @@ reiserfs_xattr_get(const struct inode *i
 	if (get_inode_sd_version(inode) == STAT_DATA_V1)
 		return -EOPNOTSUPP;
 
-	fp = open_xa_file(inode, name, FL_READONLY);
+	fp = open_xattr_file(inode, name, XATTR_REPLACE);
 	if (IS_ERR(fp)) {
 		err = PTR_ERR(fp);
 		goto out;
@@ -586,12 +517,12 @@ reiserfs_xattr_get(const struct inode *i
 	/* Just return the size needed */
 	if (buffer == NULL) {
 		err = isize - sizeof(struct reiserfs_xattr_header);
-		goto out_dput;
+		goto out_fput;
 	}
 
 	if (buffer_size < isize - sizeof(struct reiserfs_xattr_header)) {
 		err = -ERANGE;
-		goto out_dput;
+		goto out_fput;
 	}
 
 	while (file_pos < isize) {
@@ -606,7 +537,7 @@ reiserfs_xattr_get(const struct inode *i
 		page = reiserfs_get_page(xinode, file_pos >> PAGE_CACHE_SHIFT);
 		if (IS_ERR(page)) {
 			err = PTR_ERR(page);
-			goto out_dput;
+			goto out_fput;
 		}
 
 		lock_page(page);
@@ -625,7 +556,7 @@ reiserfs_xattr_get(const struct inode *i
 						 "associated with %k", name,
 						 INODE_PKEY(inode));
 				err = -EIO;
-				goto out_dput;
+				goto out_fput;
 			}
 			hash = le32_to_cpu(rxh->h_hash);
 		}
@@ -644,9 +575,10 @@ reiserfs_xattr_get(const struct inode *i
 				 "Invalid hash for xattr (%s) associated "
 				 "with %k", name, INODE_PKEY(inode));
 		err = -EIO;
+		goto out_fput;
 	}
 
-      out_dput:
+      out_fput:
 	fput(fp);
 
       out:
@@ -686,9 +618,7 @@ __reiserfs_xattr_del(struct dentry *xadi
 		return -EIO;
 	}
 
-	err = dir->i_op->unlink(dir, dentry);
-	if (!err)
-		d_delete(dentry);
+	err = vfs_unlink(xadir->d_inode, dentry);
 
       out_file:
 	dput(dentry);
@@ -702,7 +632,7 @@ int reiserfs_xattr_del(struct inode *ino
 	struct dentry *dir;
 	int err;
 
-	dir = open_xa_dir(inode, FL_READONLY);
+	dir = open_xa_dir(inode, XATTR_REPLACE);
 	if (IS_ERR(dir)) {
 		err = PTR_ERR(dir);
 		goto out;
@@ -748,7 +678,7 @@ int reiserfs_delete_xattrs(struct inode 
 		return 0;
 	}
 	reiserfs_read_lock_xattrs(inode->i_sb);
-	dir = open_xa_dir(inode, FL_READONLY);
+	dir = open_xa_dir(inode, XATTR_REPLACE);
 	reiserfs_read_unlock_xattrs(inode->i_sb);
 	if (IS_ERR(dir)) {
 		err = PTR_ERR(dir);
@@ -774,9 +704,11 @@ int reiserfs_delete_xattrs(struct inode 
 
 	/* Leftovers besides . and .. -- that's not good. */
 	if (dir->d_inode->i_nlink <= 2) {
-		root = get_xa_root(inode->i_sb, XATTR_REPLACE);
+		root = open_xa_root(inode->i_sb, XATTR_REPLACE);
 		reiserfs_write_lock_xattrs(inode->i_sb);
+		mutex_lock_nested(&root->d_inode->i_mutex, I_MUTEX_XATTR);
 		err = vfs_rmdir(root->d_inode, dir);
+		mutex_unlock(&root->d_inode->i_mutex);
 		reiserfs_write_unlock_xattrs(inode->i_sb);
 		dput(root);
 	} else {
@@ -841,7 +773,7 @@ int reiserfs_chown_xattrs(struct inode *
 		return 0;
 	}
 	reiserfs_read_lock_xattrs(inode->i_sb);
-	dir = open_xa_dir(inode, FL_READONLY);
+	dir = open_xa_dir(inode, XATTR_REPLACE);
 	reiserfs_read_unlock_xattrs(inode->i_sb);
 	if (IS_ERR(dir)) {
 		if (PTR_ERR(dir) != -ENODATA)
@@ -1038,7 +970,7 @@ ssize_t reiserfs_listxattr(struct dentry
 
 	reiserfs_read_lock_xattr_i(dentry->d_inode);
 	reiserfs_read_lock_xattrs(dentry->d_sb);
-	dir = open_xa_dir(dentry->d_inode, FL_READONLY);
+	dir = open_xa_dir(dentry->d_inode, XATTR_REPLACE);
 	reiserfs_read_unlock_xattrs(dentry->d_sb);
 	if (IS_ERR(dir)) {
 		err = PTR_ERR(dir);

-- 
Jeff Mahoney
SUSE Labs