diff options
| -rw-r--r-- | fs/reiserfs/inode.c | 14 | ||||
| -rw-r--r-- | fs/reiserfs/namei.c | 29 | ||||
| -rw-r--r-- | fs/reiserfs/super.c | 4 | ||||
| -rw-r--r-- | fs/reiserfs/xattr.c | 70 | ||||
| -rw-r--r-- | fs/reiserfs/xattr_acl.c | 74 | ||||
| -rw-r--r-- | include/linux/reiserfs_fs.h | 3 | ||||
| -rw-r--r-- | include/linux/reiserfs_fs_sb.h | 3 | ||||
| -rw-r--r-- | include/linux/reiserfs_xattr.h | 28 |
8 files changed, 74 insertions, 151 deletions
diff --git a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c index cd42a865808..50a73e7afdc 100644 --- a/fs/reiserfs/inode.c +++ b/fs/reiserfs/inode.c @@ -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; } diff --git a/fs/reiserfs/namei.c b/fs/reiserfs/namei.c index c8430f1c824..ddf1bcd41c8 100644 --- a/fs/reiserfs/namei.c +++ b/fs/reiserfs/namei.c @@ -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); @@ -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; @@ -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; } @@ -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); @@ -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); @@ -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; } @@ -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) { @@ -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); @@ -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; } diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c index fc7cb4661ee..6d10f81b4fc 100644 --- a/fs/reiserfs/super.c +++ b/fs/reiserfs/super.c @@ -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); diff --git a/fs/reiserfs/xattr.c b/fs/reiserfs/xattr.c index 57920a4df7a..62c98829c54 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 @@ 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; @@ -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: @@ -437,6 +452,9 @@ out: 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 @@ 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); @@ -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; @@ -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: @@ -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, @@ -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; } @@ -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; } @@ -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); @@ -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; } @@ -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) { @@ -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) @@ -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) { diff --git a/fs/reiserfs/xattr_acl.c b/fs/reiserfs/xattr_acl.c index 9128e4d5ba6..d63b2c5850c 100644 --- a/fs/reiserfs/xattr_acl.c +++ b/fs/reiserfs/xattr_acl.c @@ -172,6 +172,29 @@ static void *posix_acl_to_disk(const struct posix_acl *acl, size_t * size) 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 @@ struct posix_acl *reiserfs_get_acl(struct inode *inode, int type) 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 @@ struct posix_acl *reiserfs_get_acl(struct inode *inode, int type) } 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 @@ reiserfs_set_acl(struct inode *inode, int type, struct posix_acl *acl) 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 @@ int reiserfs_cache_default_acl(struct inode *inode) 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 @@ int reiserfs_acl_chmod(struct inode *inode) 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 @@ int reiserfs_acl_chmod(struct inode *inode) 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 @@ posix_acl_access_set(struct inode *inode, const char *name, 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 @@ posix_acl_default_set(struct inode *inode, const char *name, 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; } diff --git a/include/linux/reiserfs_fs.h b/include/linux/reiserfs_fs.h index 6c4af98b676..e00d240314c 100644 --- a/include/linux/reiserfs_fs.h +++ b/include/linux/reiserfs_fs.h @@ -2224,7 +2224,4 @@ int reiserfs_unpack(struct inode *inode, struct file *filp); #define reiserfs_write_lock( sb ) lock_kernel() #define reiserfs_write_unlock( sb ) unlock_kernel() -/* xattr stuff */ -#define REISERFS_XATTR_DIR_SEM(s) (REISERFS_SB(s)->xattr_dir_sem) - #endif /* _LINUX_REISER_FS_H */ diff --git a/include/linux/reiserfs_fs_sb.h b/include/linux/reiserfs_fs_sb.h index cbb8868e844..c8aee41ccc2 100644 --- a/include/linux/reiserfs_fs_sb.h +++ b/include/linux/reiserfs_fs_sb.h @@ -403,9 +403,6 @@ struct reiserfs_sb_info { spinlock_t bitmap_lock; /* this lock on now only used to protect reserved_blocks variable */ struct dentry *priv_root; /* root of /.reiserfs_priv */ struct dentry *xattr_root; /* root of /.reiserfs_priv/.xa */ -#ifdef CONFIG_REISERFS_FS_XATTR - struct rw_semaphore xattr_dir_sem; -#endif int j_errno; #ifdef CONFIG_QUOTA char *s_qf_names[MAXQUOTAS]; diff --git a/include/linux/reiserfs_xattr.h b/include/linux/reiserfs_xattr.h index 13cdd5e1cb6..65c16fa5124 100644 --- a/include/linux/reiserfs_xattr.h +++ b/include/linux/reiserfs_xattr.h @@ -67,45 +67,27 @@ 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_xattrs(struct super_block *sb) -{ - down_write(&REISERFS_XATTR_DIR_SEM(sb)); -} -static inline void reiserfs_write_unlock_xattrs(struct super_block *sb) -{ - up_write(&REISERFS_XATTR_DIR_SEM(sb)); -} -static inline void reiserfs_read_lock_xattrs(struct super_block *sb) -{ - down_read(&REISERFS_XATTR_DIR_SEM(sb)); -} - -static inline void reiserfs_read_unlock_xattrs(struct super_block *sb) -{ - up_read(&REISERFS_XATTR_DIR_SEM(sb)); -} - static inline void reiserfs_write_lock_xattr_i(struct inode *inode) { - down_write(&REISERFS_I(inode)->xattr_sem); + down_write(&REISERFS_I(inode)->i_xattr_sem); } static inline void reiserfs_write_unlock_xattr_i(struct inode *inode) { - up_write(&REISERFS_I(inode)->xattr_sem); + up_write(&REISERFS_I(inode)->i_xattr_sem); } static inline void reiserfs_read_lock_xattr_i(struct inode *inode) { - down_read(&REISERFS_I(inode)->xattr_sem); + down_read(&REISERFS_I(inode)->i_xattr_sem); } static inline void reiserfs_read_unlock_xattr_i(struct inode *inode) { - up_read(&REISERFS_I(inode)->xattr_sem); + up_read(&REISERFS_I(inode)->i_xattr_sem); } static inline void reiserfs_init_xattr_rwsem(struct inode *inode) { - init_rwsem(&REISERFS_I(inode)->xattr_sem); + init_rwsem(&REISERFS_I(inode)->i_xattr_sem); } #else |