Skip to content

Commit

Permalink
reiserfs: eliminate per-super xattr lock
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
Jeff Mahoney authored and Linus Torvalds committed Mar 30, 2009
1 parent 6c17675 commit d984561
Show file tree
Hide file tree
Showing 8 changed files with 74 additions and 151 deletions.
14 changes: 1 addition & 13 deletions fs/reiserfs/inode.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1957,19 +1957,7 @@ int reiserfs_new_inode(struct reiserfs_transaction_handle *th,
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;
}

Expand Down
29 changes: 0 additions & 29 deletions fs/reiserfs/namei.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -609,9 +609,6 @@ static int reiserfs_create(struct inode *dir, struct dentry *dentry, int mode,

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);
Expand All @@ -624,11 +621,6 @@ static int reiserfs_create(struct inode *dir, struct dentry *dentry, int mode,
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;
Expand All @@ -655,8 +647,6 @@ static int reiserfs_create(struct inode *dir, struct dentry *dentry, int mode,
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;
}
Expand Down Expand Up @@ -686,9 +676,6 @@ static int reiserfs_mknod(struct inode *dir, struct dentry *dentry, int mode,

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);
Expand All @@ -702,11 +689,6 @@ static int reiserfs_mknod(struct inode *dir, struct dentry *dentry, int mode,
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);

Expand Down Expand Up @@ -736,8 +718,6 @@ static int reiserfs_mknod(struct inode *dir, struct dentry *dentry, int mode,
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;
}
Expand Down Expand Up @@ -767,8 +747,6 @@ static int reiserfs_mkdir(struct inode *dir, struct dentry *dentry, int mode)
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) {
Expand All @@ -790,11 +768,6 @@ static int reiserfs_mkdir(struct inode *dir, struct dentry *dentry, int mode)
goto out_failed;
}

if (locked) {
reiserfs_write_unlock_xattrs(dir->i_sb);
locked = 0;
}

reiserfs_update_inode_transaction(inode);
reiserfs_update_inode_transaction(dir);

Expand Down Expand Up @@ -824,8 +797,6 @@ static int reiserfs_mkdir(struct inode *dir, struct dentry *dentry, int mode)
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;
}
Expand Down
4 changes: 0 additions & 4 deletions fs/reiserfs/super.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1646,10 +1646,6 @@ static int reiserfs_fill_super(struct super_block *s, void *data, int silent)
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);

Expand Down
70 changes: 35 additions & 35 deletions fs/reiserfs/xattr.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -392,16 +398,17 @@ 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)
{
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;
Expand All @@ -416,18 +423,26 @@ int reiserfs_delete_xattrs(struct inode *inode)
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:
Expand All @@ -437,6 +452,9 @@ int reiserfs_delete_xattrs(struct inode *inode)
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;
}

Expand Down Expand Up @@ -485,9 +503,7 @@ int reiserfs_chown_xattrs(struct inode *inode, struct iattr *attrs)
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);
Expand Down Expand Up @@ -731,6 +747,11 @@ 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);

isize = i_size_read(dentry->d_inode);
REISERFS_I(inode)->i_flags |= i_has_xattr_dir;

Expand Down Expand Up @@ -798,6 +819,7 @@ reiserfs_xattr_get(const struct inode *inode, const char *name, void *buffer,
}

out_dput:
mutex_unlock(&dentry->d_inode->i_mutex);
dput(dentry);

out:
Expand Down Expand Up @@ -834,7 +856,6 @@ int reiserfs_xattr_del(struct inode *inode, const char *name)
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,
Expand All @@ -848,9 +869,7 @@ reiserfs_getxattr(struct dentry * dentry, const char *name, void *buffer,
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;
}
Expand All @@ -866,23 +885,13 @@ reiserfs_setxattr(struct dentry *dentry, const char *name, const void *value,
{
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;
}
Expand All @@ -902,8 +911,6 @@ int reiserfs_removexattr(struct dentry *dentry, const char *name)
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);
Expand All @@ -917,7 +924,6 @@ int reiserfs_removexattr(struct dentry *dentry, const char *name)
mark_inode_dirty(dentry->d_inode);

out:
reiserfs_read_unlock_xattrs(dentry->d_sb);
reiserfs_write_unlock_xattr_i(dentry->d_inode);
return err;
}
Expand Down Expand Up @@ -966,8 +972,6 @@ reiserfs_listxattr_filler(void *buf, const char *name, int namelen,

/*
* Inode operation listxattr()
*
* Preliminary locking: we down dentry->d_inode->i_mutex
*/
ssize_t reiserfs_listxattr(struct dentry * dentry, char *buffer, size_t size)
{
Expand All @@ -983,9 +987,7 @@ ssize_t reiserfs_listxattr(struct dentry * dentry, char *buffer, size_t size)
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)
Expand Down Expand Up @@ -1114,11 +1116,9 @@ static int reiserfs_check_acl(struct inode *inode, int mask)
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) {
Expand Down
Loading

0 comments on commit d984561

Please sign in to comment.