reiserfs: make per-inode xattr locking more fine grained

The per-inode locking can be made more fine-grained to surround just the
interaction with the filesystem itself.  This really only applies to
protecting reads during a write, since concurrent writes are barred with
inode->i_mutex at the vfs level.

Signed-off-by: Jeff Mahoney <jeffm@suse.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

4 files changed, 55 insertions, 90 deletions

diff --git a/fs/reiserfs/xattr.c b/fs/reiserfs/xattr.c
index 62c98829c545..ccb8e4d4c032 100644
--- a/fs/reiserfs/xattr.c
+++ b/fs/reiserfs/xattr.c
@@ -29,10 +29,8 @@
  * to the inode so that unnecessary lookups are avoided.
  *
  * Locking works like so:
- * The xattr root (/.reiserfs_priv/xattrs) is protected by its i_mutex.
- * The xattr dir (/.reiserfs_priv/xattrs/<oid>.<gen>) is protected by
- * inode->xattr_sem.
- * The xattrs themselves are likewise protected by the xattr_sem.
+ * Directory components (xattr root, xattr dir) are protectd by their i_mutex.
+ * The xattrs themselves are protected by the xattr_sem.
  */
 
 #include <linux/reiserfs_fs.h>
@@ -55,6 +53,8 @@
 #define PRIVROOT_NAME ".reiserfs_priv"
 #define XAROOT_NAME   "xattrs"
 
+static struct reiserfs_xattr_handler *find_xattr_handler_prefix(const char *);
+
 /* Helpers for inode ops. We do this so that we don't have all the VFS
  * overhead and also for proper i_mutex annotation.
  * dir->i_mutex must be held for all of them. */
@@ -339,12 +339,14 @@ int xattr_readdir(struct inode *inode, filldir_t filler, void *buf)
         return res;
 }
 
+/* expects xadir->d_inode->i_mutex to be locked */
 static int
 __reiserfs_xattr_del(struct dentry *xadir, const char *name, int namelen)
 {
         struct dentry *dentry;
         struct inode *dir = xadir->d_inode;
         int err = 0;
+        struct reiserfs_xattr_handler *xah;
 
         dentry = lookup_one_len(name, xadir, namelen);
         if (IS_ERR(dentry)) {
@@ -372,6 +374,14 @@ __reiserfs_xattr_del(struct dentry *xadir, const char *name, int namelen)
                 return -EIO;
         }
 
+        /* Deletion pre-operation */
+        xah = find_xattr_handler_prefix(name);
+        if (xah && xah->del) {
+                err = xah->del(dentry->d_inode, name);
+                if (err)
+                        goto out;
+        }
+
         err = xattr_unlink(dir, dentry);
 
 out_file:
@@ -398,7 +408,7 @@ reiserfs_delete_xattrs_filler(void *buf, const char *name, int namelen,
 /* This is called w/ inode->i_mutex downed */
 int reiserfs_delete_xattrs(struct inode *inode)
 {
-        int err = 0;
+        int err = -ENODATA;
         struct dentry *dir, *root;
         struct reiserfs_transaction_handle th;
         int blocks = JOURNAL_PER_BALANCE_CNT * 2 + 2 +
@@ -414,14 +424,19 @@ int reiserfs_delete_xattrs(struct inode *inode)
                 goto out;
         } else if (!dir->d_inode) {
                 dput(dir);
-                return 0;
+                goto out;
         }
 
         mutex_lock_nested(&dir->d_inode->i_mutex, I_MUTEX_XATTR);
         err = xattr_readdir(dir->d_inode, reiserfs_delete_xattrs_filler, dir);
         mutex_unlock(&dir->d_inode->i_mutex);
-        if (err)
-                goto out_dir;
+        if (err) {
+                dput(dir);
+                goto out;
+        }
+
+        root = dget(dir->d_parent);
+        dput(dir);
 
         /* We start a transaction here to avoid a ABBA situation
          * between the xattr root's i_mutex and the journal lock.
@@ -435,19 +450,14 @@ int reiserfs_delete_xattrs(struct inode *inode)
         err = journal_begin(&th, inode->i_sb, blocks);
         if (!err) {
                 int jerror;
-                root = dget(dir->d_parent);
                 mutex_lock_nested(&root->d_inode->i_mutex, I_MUTEX_XATTR);
                 err = xattr_rmdir(root->d_inode, dir);
                 jerror = journal_end(&th, inode->i_sb, blocks);
                 mutex_unlock(&root->d_inode->i_mutex);
-                dput(root);
-
                 err = jerror ?: err;
         }
 
-out_dir:
-        dput(dir);
-
+        dput(root);
 out:
         if (!err)
                 REISERFS_I(inode)->i_flags =
@@ -484,7 +494,7 @@ reiserfs_chown_xattrs_filler(void *buf, const char *name, int namelen,
 
         if (!S_ISDIR(xafile->d_inode->i_mode)) {
                 mutex_lock_nested(&xafile->d_inode->i_mutex, I_MUTEX_CHILD);
-                err = notify_change(xafile, attrs);
+                err = reiserfs_setattr(xafile, attrs);
                 mutex_unlock(&xafile->d_inode->i_mutex);
         }
         dput(xafile);
@@ -520,13 +530,16 @@ int reiserfs_chown_xattrs(struct inode *inode, struct iattr *attrs)
         err = xattr_readdir(dir->d_inode, reiserfs_chown_xattrs_filler, &buf);
 
         if (!err)
-                err = notify_change(dir, attrs);
+                err = reiserfs_setattr(dir, attrs);
         mutex_unlock(&dir->d_inode->i_mutex);
 
         attrs->ia_valid = ia_valid;
 out_dir:
         dput(dir);
 out:
+        if (err)
+                reiserfs_warning(inode->i_sb, "jdm-20007",
+                                 "Couldn't chown all xattrs (%d)\n", err);
         return err;
 }
 
@@ -635,9 +648,8 @@ reiserfs_xattr_set(struct inode *inode, const char *name, const void *buffer,
         if (get_inode_sd_version(inode) == STAT_DATA_V1)
                 return -EOPNOTSUPP;
 
-        /* Empty xattrs are ok, they're just empty files, no hash */
-        if (buffer && buffer_size)
-                xahash = xattr_hash(buffer, buffer_size);
+        if (!buffer)
+                return reiserfs_xattr_del(inode, name);
 
         dentry = get_xa_file_dentry(inode, name, flags);
         if (IS_ERR(dentry)) {
@@ -645,13 +657,19 @@ reiserfs_xattr_set(struct inode *inode, const char *name, const void *buffer,
                 goto out;
         }
 
+        down_write(&REISERFS_I(inode)->i_xattr_sem);
+
+        xahash = xattr_hash(buffer, buffer_size);
         REISERFS_I(inode)->i_flags |= i_has_xattr_dir;
 
         /* Resize it so we're ok to write there */
         newattrs.ia_size = buffer_size;
+        newattrs.ia_ctime = current_fs_time(inode->i_sb);
         newattrs.ia_valid = ATTR_SIZE | ATTR_CTIME;
         mutex_lock_nested(&dentry->d_inode->i_mutex, I_MUTEX_XATTR);
-        err = notify_change(dentry, &newattrs);
+        down_write(&dentry->d_inode->i_alloc_sem);
+        err = reiserfs_setattr(dentry, &newattrs);
+        up_write(&dentry->d_inode->i_alloc_sem);
         mutex_unlock(&dentry->d_inode->i_mutex);
         if (err)
                 goto out_filp;
@@ -712,6 +730,7 @@ reiserfs_xattr_set(struct inode *inode, const char *name, const void *buffer,
         }
 
       out_filp:
+        up_write(&REISERFS_I(inode)->i_xattr_sem);
         dput(dentry);
 
       out:
@@ -747,10 +766,7 @@ reiserfs_xattr_get(const struct inode *inode, const char *name, void *buffer,
                 goto out;
         }
 
-        /* protect against concurrent access. xattrs are backed by
-         * regular files, but they're not regular files. The updates
-         * must be atomic from the perspective of the user. */
-        mutex_lock_nested(&dentry->d_inode->i_mutex, I_MUTEX_XATTR);
+        down_read(&REISERFS_I(inode)->i_xattr_sem);
 
         isize = i_size_read(dentry->d_inode);
         REISERFS_I(inode)->i_flags |= i_has_xattr_dir;
@@ -758,12 +774,12 @@ reiserfs_xattr_get(const struct inode *inode, const char *name, void *buffer,
         /* Just return the size needed */
         if (buffer == NULL) {
                 err = isize - sizeof(struct reiserfs_xattr_header);
-                goto out_dput;
+                goto out_unlock;
         }
 
         if (buffer_size < isize - sizeof(struct reiserfs_xattr_header)) {
                 err = -ERANGE;
-                goto out_dput;
+                goto out_unlock;
         }
 
         while (file_pos < isize) {
@@ -778,7 +794,7 @@ reiserfs_xattr_get(const struct inode *inode, const char *name, void *buffer,
                 page = reiserfs_get_page(dentry->d_inode, file_pos);
                 if (IS_ERR(page)) {
                         err = PTR_ERR(page);
-                        goto out_dput;
+                        goto out_unlock;
                 }
 
                 lock_page(page);
@@ -797,7 +813,7 @@ reiserfs_xattr_get(const struct inode *inode, const char *name, void *buffer,
                                                  "associated with %k", name,
                                                  INODE_PKEY(inode));
                                 err = -EIO;
-                                goto out_dput;
+                                goto out_unlock;
                         }
                         hash = le32_to_cpu(rxh->h_hash);
                 }
@@ -818,8 +834,8 @@ reiserfs_xattr_get(const struct inode *inode, const char *name, void *buffer,
                 err = -EIO;
         }
 
-out_dput:
-        mutex_unlock(&dentry->d_inode->i_mutex);
+out_unlock:
+        up_read(&REISERFS_I(inode)->i_xattr_sem);
         dput(dentry);
 
 out:
@@ -852,8 +868,6 @@ int reiserfs_xattr_del(struct inode *inode, const char *name)
 }
 
 /* Actual operations that are exported to VFS-land */
-
-static struct reiserfs_xattr_handler *find_xattr_handler_prefix(const char *);
 /*
  * Inode operation getxattr()
  */
@@ -868,9 +882,7 @@ reiserfs_getxattr(struct dentry * dentry, const char *name, void *buffer,
             get_inode_sd_version(dentry->d_inode) == STAT_DATA_V1)
                 return -EOPNOTSUPP;
 
-        reiserfs_read_lock_xattr_i(dentry->d_inode);
         err = xah->get(dentry->d_inode, name, buffer, size);
-        reiserfs_read_unlock_xattr_i(dentry->d_inode);
         return err;
 }
 
@@ -890,9 +902,7 @@ reiserfs_setxattr(struct dentry *dentry, const char *name, const void *value,
             get_inode_sd_version(dentry->d_inode) == STAT_DATA_V1)
                 return -EOPNOTSUPP;
 
-        reiserfs_write_lock_xattr_i(dentry->d_inode);
         err = xah->set(dentry->d_inode, name, value, size, flags);
-        reiserfs_write_unlock_xattr_i(dentry->d_inode);
         return err;
 }
 
@@ -910,21 +920,11 @@ int reiserfs_removexattr(struct dentry *dentry, const char *name)
             get_inode_sd_version(dentry->d_inode) == STAT_DATA_V1)
                 return -EOPNOTSUPP;
 
-        reiserfs_write_lock_xattr_i(dentry->d_inode);
-        /* Deletion pre-operation */
-        if (xah->del) {
-                err = xah->del(dentry->d_inode, name);
-                if (err)
-                        goto out;
-        }
-
         err = reiserfs_xattr_del(dentry->d_inode, name);
 
         dentry->d_inode->i_ctime = CURRENT_TIME_SEC;
         mark_inode_dirty(dentry->d_inode);
 
-      out:
-        reiserfs_write_unlock_xattr_i(dentry->d_inode);
         return err;
 }
 
@@ -986,7 +986,6 @@ ssize_t reiserfs_listxattr(struct dentry * dentry, char *buffer, size_t size)
             get_inode_sd_version(dentry->d_inode) == STAT_DATA_V1)
                 return -EOPNOTSUPP;
 
-        reiserfs_read_lock_xattr_i(dentry->d_inode);
         dir = open_xa_dir(dentry->d_inode, XATTR_REPLACE);
         if (IS_ERR(dir)) {
                 err = PTR_ERR(dir);
@@ -1005,19 +1004,16 @@ ssize_t reiserfs_listxattr(struct dentry * dentry, char *buffer, size_t size)
         mutex_lock_nested(&dir->d_inode->i_mutex, I_MUTEX_XATTR);
         err = xattr_readdir(dir->d_inode, reiserfs_listxattr_filler, &buf);
         mutex_unlock(&dir->d_inode->i_mutex);
-        if (err)
-                goto out_dir;
 
-        if (buf.r_pos > buf.r_size && buffer != NULL)
-                err = -ERANGE;
-        else
-                err = buf.r_pos;
+        if (!err) {
+                if (buf.r_pos > buf.r_size && buffer != NULL)
+                        err = -ERANGE;
+                else
+                        err = buf.r_pos;
+        }
 
-      out_dir:
         dput(dir);
-
-      out:
-        reiserfs_read_unlock_xattr_i(dentry->d_inode);
+out:
         return err;
 }
 
@@ -1115,12 +1111,8 @@ static int reiserfs_check_acl(struct inode *inode, int mask)
         struct posix_acl *acl;
         int error = -EAGAIN; /* do regular unix permission checks by default */
 
-        reiserfs_read_lock_xattr_i(inode);
-
         acl = reiserfs_get_acl(inode, ACL_TYPE_ACCESS);
 
-        reiserfs_read_unlock_xattr_i(inode);
-
         if (acl) {
                 if (!IS_ERR(acl)) {
                         error = posix_acl_permission(inode, acl, mask);
diff --git a/fs/reiserfs/xattr_acl.c b/fs/reiserfs/xattr_acl.c
index d63b2c5850c3..d3ce6ee9b262 100644
--- a/fs/reiserfs/xattr_acl.c
+++ b/fs/reiserfs/xattr_acl.c
@@ -418,9 +418,7 @@ int reiserfs_cache_default_acl(struct inode *inode)
         int ret = 0;
         if (reiserfs_posixacl(inode->i_sb) && !IS_PRIVATE(inode)) {
                 struct posix_acl *acl;
-                reiserfs_read_lock_xattr_i(inode);
                 acl = reiserfs_get_acl(inode, ACL_TYPE_DEFAULT);
-                reiserfs_read_unlock_xattr_i(inode);
                 ret = (acl && !IS_ERR(acl));
                 if (ret)
                         posix_acl_release(acl);
@@ -452,11 +450,8 @@ int reiserfs_acl_chmod(struct inode *inode)
         if (!clone)
                 return -ENOMEM;
         error = posix_acl_chmod_masq(clone, inode->i_mode);
-        if (!error) {
-                reiserfs_write_lock_xattr_i(inode);
+        if (!error)
                 error = reiserfs_set_acl(inode, ACL_TYPE_ACCESS, clone);
-                reiserfs_write_unlock_xattr_i(inode);
-        }
         posix_acl_release(clone);
         return error;
 }
diff --git a/include/linux/reiserfs_fs_i.h b/include/linux/reiserfs_fs_i.h
index 201dd910b042..76360b36ac33 100644
--- a/include/linux/reiserfs_fs_i.h
+++ b/include/linux/reiserfs_fs_i.h
@@ -59,7 +59,7 @@ struct reiserfs_inode_info {
         struct posix_acl *i_acl_default;
 #endif
 #ifdef CONFIG_REISERFS_FS_XATTR
-        struct rw_semaphore xattr_sem;
+        struct rw_semaphore i_xattr_sem;
 #endif
         struct inode vfs_inode;
 };
diff --git a/include/linux/reiserfs_xattr.h b/include/linux/reiserfs_xattr.h
index 65c16fa51246..3bd154fd56e9 100644
--- a/include/linux/reiserfs_xattr.h
+++ b/include/linux/reiserfs_xattr.h
@@ -67,24 +67,6 @@ extern struct reiserfs_xattr_handler user_handler;
 extern struct reiserfs_xattr_handler trusted_handler;
 extern struct reiserfs_xattr_handler security_handler;
 
-static inline void reiserfs_write_lock_xattr_i(struct inode *inode)
-{
-        down_write(&REISERFS_I(inode)->i_xattr_sem);
-}
-static inline void reiserfs_write_unlock_xattr_i(struct inode *inode)
-{
-        up_write(&REISERFS_I(inode)->i_xattr_sem);
-}
-static inline void reiserfs_read_lock_xattr_i(struct inode *inode)
-{
-        down_read(&REISERFS_I(inode)->i_xattr_sem);
-}
-
-static inline void reiserfs_read_unlock_xattr_i(struct inode *inode)
-{
-        up_read(&REISERFS_I(inode)->i_xattr_sem);
-}
-
 static inline void reiserfs_init_xattr_rwsem(struct inode *inode)
 {
         init_rwsem(&REISERFS_I(inode)->i_xattr_sem);
@@ -96,10 +78,6 @@ static inline void reiserfs_init_xattr_rwsem(struct inode *inode)
 #define reiserfs_setxattr NULL
 #define reiserfs_listxattr NULL
 #define reiserfs_removexattr NULL
-#define reiserfs_write_lock_xattrs(sb) do {;} while(0)
-#define reiserfs_write_unlock_xattrs(sb) do {;} while(0)
-#define reiserfs_read_lock_xattrs(sb)
-#define reiserfs_read_unlock_xattrs(sb)
 
 #define reiserfs_permission NULL