reiserfs: eliminate per-super xattr lock
With the switch to using inode->i_mutex locking during lookups/creation
in the xattr root, the per-super xattr lock is no longer needed.
This patch removes it.
Signed-off-by: Jeff Mahoney <jeffm@suse.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
diff --git a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c
index cd42a86..50a73e7 100644
--- a/fs/reiserfs/inode.c
+++ b/fs/reiserfs/inode.c
@@ -1957,19 +1957,7 @@
inode->i_nlink = 0;
th->t_trans_id = 0; /* so the caller can't use this handle later */
unlock_new_inode(inode); /* OK to do even if we hadn't locked it */
-
- /* If we were inheriting an ACL, we need to release the lock so that
- * iput doesn't deadlock in reiserfs_delete_xattrs. The locking
- * code really needs to be reworked, but this will take care of it
- * for now. -jeffm */
-#ifdef CONFIG_REISERFS_FS_POSIX_ACL
- if (REISERFS_I(dir)->i_acl_default && !IS_ERR(REISERFS_I(dir)->i_acl_default)) {
- reiserfs_write_unlock_xattrs(dir->i_sb);
- iput(inode);
- reiserfs_write_lock_xattrs(dir->i_sb);
- } else
-#endif
- iput(inode);
+ iput(inode);
return err;
}
diff --git a/fs/reiserfs/namei.c b/fs/reiserfs/namei.c
index c8430f1..ddf1bcd 100644
--- a/fs/reiserfs/namei.c
+++ b/fs/reiserfs/namei.c
@@ -609,9 +609,6 @@
reiserfs_write_lock(dir->i_sb);
- if (locked)
- reiserfs_write_lock_xattrs(dir->i_sb);
-
retval = journal_begin(&th, dir->i_sb, jbegin_count);
if (retval) {
drop_new_inode(inode);
@@ -624,11 +621,6 @@
if (retval)
goto out_failed;
- if (locked) {
- reiserfs_write_unlock_xattrs(dir->i_sb);
- locked = 0;
- }
-
inode->i_op = &reiserfs_file_inode_operations;
inode->i_fop = &reiserfs_file_operations;
inode->i_mapping->a_ops = &reiserfs_address_space_operations;
@@ -655,8 +647,6 @@
retval = journal_end(&th, dir->i_sb, jbegin_count);
out_failed:
- if (locked)
- reiserfs_write_unlock_xattrs(dir->i_sb);
reiserfs_write_unlock(dir->i_sb);
return retval;
}
@@ -686,9 +676,6 @@
reiserfs_write_lock(dir->i_sb);
- if (locked)
- reiserfs_write_lock_xattrs(dir->i_sb);
-
retval = journal_begin(&th, dir->i_sb, jbegin_count);
if (retval) {
drop_new_inode(inode);
@@ -702,11 +689,6 @@
goto out_failed;
}
- if (locked) {
- reiserfs_write_unlock_xattrs(dir->i_sb);
- locked = 0;
- }
-
inode->i_op = &reiserfs_special_inode_operations;
init_special_inode(inode, inode->i_mode, rdev);
@@ -736,8 +718,6 @@
retval = journal_end(&th, dir->i_sb, jbegin_count);
out_failed:
- if (locked)
- reiserfs_write_unlock_xattrs(dir->i_sb);
reiserfs_write_unlock(dir->i_sb);
return retval;
}
@@ -767,8 +747,6 @@
locked = reiserfs_cache_default_acl(dir);
reiserfs_write_lock(dir->i_sb);
- if (locked)
- reiserfs_write_lock_xattrs(dir->i_sb);
retval = journal_begin(&th, dir->i_sb, jbegin_count);
if (retval) {
@@ -790,11 +768,6 @@
goto out_failed;
}
- if (locked) {
- reiserfs_write_unlock_xattrs(dir->i_sb);
- locked = 0;
- }
-
reiserfs_update_inode_transaction(inode);
reiserfs_update_inode_transaction(dir);
@@ -824,8 +797,6 @@
unlock_new_inode(inode);
retval = journal_end(&th, dir->i_sb, jbegin_count);
out_failed:
- if (locked)
- reiserfs_write_unlock_xattrs(dir->i_sb);
reiserfs_write_unlock(dir->i_sb);
return retval;
}
diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c
index fc7cb46..6d10f81 100644
--- a/fs/reiserfs/super.c
+++ b/fs/reiserfs/super.c
@@ -1646,10 +1646,6 @@
REISERFS_SB(s)->s_alloc_options.preallocmin = 0;
/* Preallocate by 16 blocks (17-1) at once */
REISERFS_SB(s)->s_alloc_options.preallocsize = 17;
-#ifdef CONFIG_REISERFS_FS_XATTR
- /* Initialize the rwsem for xattr dir */
- init_rwsem(&REISERFS_SB(s)->xattr_dir_sem);
-#endif
/* setup default block allocator options */
reiserfs_init_alloc_options(s);
diff --git a/fs/reiserfs/xattr.c b/fs/reiserfs/xattr.c
index 57920a4..62c9882 100644
--- a/fs/reiserfs/xattr.c
+++ b/fs/reiserfs/xattr.c
@@ -27,6 +27,12 @@
* these are special cases for filesystem ACLs, they are interpreted by the
* kernel, in addition, they are negatively and positively cached and attached
* 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.
*/
#include <linux/reiserfs_fs.h>
@@ -392,16 +398,17 @@
/* This is called w/ inode->i_mutex downed */
int reiserfs_delete_xattrs(struct inode *inode)
{
- struct dentry *dir, *root;
int err = 0;
+ struct dentry *dir, *root;
+ struct reiserfs_transaction_handle th;
+ int blocks = JOURNAL_PER_BALANCE_CNT * 2 + 2 +
+ 4 * REISERFS_QUOTA_TRANS_BLOCKS(inode->i_sb);
/* Skip out, an xattr has no xattrs associated with it */
if (IS_PRIVATE(inode) || get_inode_sd_version(inode) == STAT_DATA_V1)
return 0;
- reiserfs_read_lock_xattrs(inode->i_sb);
dir = open_xa_dir(inode, XATTR_REPLACE);
- reiserfs_read_unlock_xattrs(inode->i_sb);
if (IS_ERR(dir)) {
err = PTR_ERR(dir);
goto out;
@@ -416,18 +423,26 @@
if (err)
goto out_dir;
- /* Leftovers besides . and .. -- that's not good. */
- if (dir->d_inode->i_nlink <= 2) {
- root = open_xa_root(inode->i_sb, XATTR_REPLACE);
- reiserfs_write_lock_xattrs(inode->i_sb);
+ /* We start a transaction here to avoid a ABBA situation
+ * between the xattr root's i_mutex and the journal lock.
+ * Inode creation will inherit an ACL, which requires a
+ * lookup. The lookup locks the xattr root i_mutex with a
+ * transaction open. Inode deletion takes teh xattr root
+ * i_mutex to delete the directory and then starts a
+ * transaction inside it. Boom. This doesn't incur much
+ * additional overhead since the reiserfs_rmdir transaction
+ * will just nest inside the outer transaction. */
+ 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);
- reiserfs_write_unlock_xattrs(inode->i_sb);
dput(root);
- } else {
- reiserfs_warning(inode->i_sb, "jdm-20006",
- "Couldn't remove all entries in directory");
+
+ err = jerror ?: err;
}
out_dir:
@@ -437,6 +452,9 @@
if (!err)
REISERFS_I(inode)->i_flags =
REISERFS_I(inode)->i_flags & ~i_has_xattr_dir;
+ else
+ reiserfs_warning(inode->i_sb, "jdm-20004",
+ "Couldn't remove all xattrs (%d)\n", err);
return err;
}
@@ -485,9 +503,7 @@
if (IS_PRIVATE(inode) || get_inode_sd_version(inode) == STAT_DATA_V1)
return 0;
- reiserfs_read_lock_xattrs(inode->i_sb);
dir = open_xa_dir(inode, XATTR_REPLACE);
- reiserfs_read_unlock_xattrs(inode->i_sb);
if (IS_ERR(dir)) {
if (PTR_ERR(dir) != -ENODATA)
err = PTR_ERR(dir);
@@ -731,6 +747,11 @@
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);
+
isize = i_size_read(dentry->d_inode);
REISERFS_I(inode)->i_flags |= i_has_xattr_dir;
@@ -798,6 +819,7 @@
}
out_dput:
+ mutex_unlock(&dentry->d_inode->i_mutex);
dput(dentry);
out:
@@ -834,7 +856,6 @@
static struct reiserfs_xattr_handler *find_xattr_handler_prefix(const char *);
/*
* Inode operation getxattr()
- * Preliminary locking: we down dentry->d_inode->i_mutex
*/
ssize_t
reiserfs_getxattr(struct dentry * dentry, const char *name, void *buffer,
@@ -848,9 +869,7 @@
return -EOPNOTSUPP;
reiserfs_read_lock_xattr_i(dentry->d_inode);
- reiserfs_read_lock_xattrs(dentry->d_sb);
err = xah->get(dentry->d_inode, name, buffer, size);
- reiserfs_read_unlock_xattrs(dentry->d_sb);
reiserfs_read_unlock_xattr_i(dentry->d_inode);
return err;
}
@@ -866,23 +885,13 @@
{
struct reiserfs_xattr_handler *xah = find_xattr_handler_prefix(name);
int err;
- int lock;
if (!xah || !reiserfs_xattrs(dentry->d_sb) ||
get_inode_sd_version(dentry->d_inode) == STAT_DATA_V1)
return -EOPNOTSUPP;
reiserfs_write_lock_xattr_i(dentry->d_inode);
- lock = !has_xattr_dir(dentry->d_inode);
- if (lock)
- reiserfs_write_lock_xattrs(dentry->d_sb);
- else
- reiserfs_read_lock_xattrs(dentry->d_sb);
err = xah->set(dentry->d_inode, name, value, size, flags);
- if (lock)
- reiserfs_write_unlock_xattrs(dentry->d_sb);
- else
- reiserfs_read_unlock_xattrs(dentry->d_sb);
reiserfs_write_unlock_xattr_i(dentry->d_inode);
return err;
}
@@ -902,8 +911,6 @@
return -EOPNOTSUPP;
reiserfs_write_lock_xattr_i(dentry->d_inode);
- reiserfs_read_lock_xattrs(dentry->d_sb);
-
/* Deletion pre-operation */
if (xah->del) {
err = xah->del(dentry->d_inode, name);
@@ -917,7 +924,6 @@
mark_inode_dirty(dentry->d_inode);
out:
- reiserfs_read_unlock_xattrs(dentry->d_sb);
reiserfs_write_unlock_xattr_i(dentry->d_inode);
return err;
}
@@ -966,8 +972,6 @@
/*
* Inode operation listxattr()
- *
- * Preliminary locking: we down dentry->d_inode->i_mutex
*/
ssize_t reiserfs_listxattr(struct dentry * dentry, char *buffer, size_t size)
{
@@ -983,9 +987,7 @@
return -EOPNOTSUPP;
reiserfs_read_lock_xattr_i(dentry->d_inode);
- reiserfs_read_lock_xattrs(dentry->d_sb);
dir = open_xa_dir(dentry->d_inode, XATTR_REPLACE);
- reiserfs_read_unlock_xattrs(dentry->d_sb);
if (IS_ERR(dir)) {
err = PTR_ERR(dir);
if (err == -ENODATA)
@@ -1114,11 +1116,9 @@
int error = -EAGAIN; /* do regular unix permission checks by default */
reiserfs_read_lock_xattr_i(inode);
- reiserfs_read_lock_xattrs(inode->i_sb);
acl = reiserfs_get_acl(inode, ACL_TYPE_ACCESS);
- reiserfs_read_unlock_xattrs(inode->i_sb);
reiserfs_read_unlock_xattr_i(inode);
if (acl) {
diff --git a/fs/reiserfs/xattr_acl.c b/fs/reiserfs/xattr_acl.c
index 9128e4d..d63b2c5 100644
--- a/fs/reiserfs/xattr_acl.c
+++ b/fs/reiserfs/xattr_acl.c
@@ -172,6 +172,29 @@
return ERR_PTR(-EINVAL);
}
+static inline void iset_acl(struct inode *inode, struct posix_acl **i_acl,
+ struct posix_acl *acl)
+{
+ spin_lock(&inode->i_lock);
+ if (*i_acl != ERR_PTR(-ENODATA))
+ posix_acl_release(*i_acl);
+ *i_acl = posix_acl_dup(acl);
+ spin_unlock(&inode->i_lock);
+}
+
+static inline struct posix_acl *iget_acl(struct inode *inode,
+ struct posix_acl **i_acl)
+{
+ struct posix_acl *acl = ERR_PTR(-ENODATA);
+
+ spin_lock(&inode->i_lock);
+ if (*i_acl != ERR_PTR(-ENODATA))
+ acl = posix_acl_dup(*i_acl);
+ spin_unlock(&inode->i_lock);
+
+ return acl;
+}
+
/*
* Inode operation get_posix_acl().
*
@@ -199,11 +222,11 @@
return ERR_PTR(-EINVAL);
}
- if (IS_ERR(*p_acl)) {
- if (PTR_ERR(*p_acl) == -ENODATA)
- return NULL;
- } else if (*p_acl != NULL)
- return posix_acl_dup(*p_acl);
+ acl = iget_acl(inode, p_acl);
+ if (acl && !IS_ERR(acl))
+ return acl;
+ else if (PTR_ERR(acl) == -ENODATA)
+ return NULL;
size = reiserfs_xattr_get(inode, name, NULL, 0);
if (size < 0) {
@@ -229,7 +252,7 @@
} else {
acl = posix_acl_from_disk(value, retval);
if (!IS_ERR(acl))
- *p_acl = posix_acl_dup(acl);
+ iset_acl(inode, p_acl, acl);
}
kfree(value);
@@ -300,16 +323,8 @@
kfree(value);
- if (!error) {
- /* Release the old one */
- if (!IS_ERR(*p_acl) && *p_acl)
- posix_acl_release(*p_acl);
-
- if (acl == NULL)
- *p_acl = ERR_PTR(-ENODATA);
- else
- *p_acl = posix_acl_dup(acl);
- }
+ if (!error)
+ iset_acl(inode, p_acl, acl);
return error;
}
@@ -404,9 +419,7 @@
if (reiserfs_posixacl(inode->i_sb) && !IS_PRIVATE(inode)) {
struct posix_acl *acl;
reiserfs_read_lock_xattr_i(inode);
- reiserfs_read_lock_xattrs(inode->i_sb);
acl = reiserfs_get_acl(inode, ACL_TYPE_DEFAULT);
- reiserfs_read_unlock_xattrs(inode->i_sb);
reiserfs_read_unlock_xattr_i(inode);
ret = (acl && !IS_ERR(acl));
if (ret)
@@ -429,9 +442,7 @@
return 0;
}
- reiserfs_read_lock_xattrs(inode->i_sb);
acl = reiserfs_get_acl(inode, ACL_TYPE_ACCESS);
- reiserfs_read_unlock_xattrs(inode->i_sb);
if (!acl)
return 0;
if (IS_ERR(acl))
@@ -442,17 +453,8 @@
return -ENOMEM;
error = posix_acl_chmod_masq(clone, inode->i_mode);
if (!error) {
- int lock = !has_xattr_dir(inode);
reiserfs_write_lock_xattr_i(inode);
- if (lock)
- reiserfs_write_lock_xattrs(inode->i_sb);
- else
- reiserfs_read_lock_xattrs(inode->i_sb);
error = reiserfs_set_acl(inode, ACL_TYPE_ACCESS, clone);
- if (lock)
- reiserfs_write_unlock_xattrs(inode->i_sb);
- else
- reiserfs_read_unlock_xattrs(inode->i_sb);
reiserfs_write_unlock_xattr_i(inode);
}
posix_acl_release(clone);
@@ -480,14 +482,9 @@
static int posix_acl_access_del(struct inode *inode, const char *name)
{
struct reiserfs_inode_info *reiserfs_i = REISERFS_I(inode);
- struct posix_acl **acl = &reiserfs_i->i_acl_access;
if (strlen(name) != sizeof(POSIX_ACL_XATTR_ACCESS) - 1)
return -EINVAL;
- if (!IS_ERR(*acl) && *acl) {
- posix_acl_release(*acl);
- *acl = ERR_PTR(-ENODATA);
- }
-
+ iset_acl(inode, &reiserfs_i->i_acl_access, ERR_PTR(-ENODATA));
return 0;
}
@@ -533,14 +530,9 @@
static int posix_acl_default_del(struct inode *inode, const char *name)
{
struct reiserfs_inode_info *reiserfs_i = REISERFS_I(inode);
- struct posix_acl **acl = &reiserfs_i->i_acl_default;
if (strlen(name) != sizeof(POSIX_ACL_XATTR_DEFAULT) - 1)
return -EINVAL;
- if (!IS_ERR(*acl) && *acl) {
- posix_acl_release(*acl);
- *acl = ERR_PTR(-ENODATA);
- }
-
+ iset_acl(inode, &reiserfs_i->i_acl_default, ERR_PTR(-ENODATA));
return 0;
}