Skip to content

Commit

Permalink
Fix reiserfs_file_release()
Browse files Browse the repository at this point in the history
a) count file openers correctly; i_count use was completely wrong
b) use new mutex for exclusion between final close/open/truncate,
to protect tailpacking logics.  i_mutex use was wrong and resulted
in deadlocks.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
  • Loading branch information
Al Viro committed Aug 9, 2010
1 parent 918377b commit 0e4f6a7
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 26 deletions.
50 changes: 28 additions & 22 deletions fs/reiserfs/file.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,20 +38,24 @@ static int reiserfs_file_release(struct inode *inode, struct file *filp)

BUG_ON(!S_ISREG(inode->i_mode));

if (atomic_add_unless(&REISERFS_I(inode)->openers, -1, 1))
return 0;

mutex_lock(&(REISERFS_I(inode)->tailpack));

if (!atomic_dec_and_test(&REISERFS_I(inode)->openers)) {
mutex_unlock(&(REISERFS_I(inode)->tailpack));
return 0;
}

/* fast out for when nothing needs to be done */
if ((atomic_read(&inode->i_count) > 1 ||
!(REISERFS_I(inode)->i_flags & i_pack_on_close_mask) ||
if ((!(REISERFS_I(inode)->i_flags & i_pack_on_close_mask) ||
!tail_has_to_be_packed(inode)) &&
REISERFS_I(inode)->i_prealloc_count <= 0) {
mutex_unlock(&(REISERFS_I(inode)->tailpack));
return 0;
}

mutex_lock(&inode->i_mutex);

mutex_lock(&(REISERFS_I(inode)->i_mmap));
if (REISERFS_I(inode)->i_flags & i_ever_mapped)
REISERFS_I(inode)->i_flags &= ~i_pack_on_close_mask;

reiserfs_write_lock(inode->i_sb);
/* freeing preallocation only involves relogging blocks that
* are already in the current transaction. preallocation gets
Expand Down Expand Up @@ -94,37 +98,39 @@ static int reiserfs_file_release(struct inode *inode, struct file *filp)
if (!err)
err = jbegin_failure;

if (!err && atomic_read(&inode->i_count) <= 1 &&
if (!err &&
(REISERFS_I(inode)->i_flags & i_pack_on_close_mask) &&
tail_has_to_be_packed(inode)) {

/* if regular file is released by last holder and it has been
appended (we append by unformatted node only) or its direct
item(s) had to be converted, then it may have to be
indirect2direct converted */
err = reiserfs_truncate_file(inode, 0);
}
out:
mutex_unlock(&(REISERFS_I(inode)->i_mmap));
mutex_unlock(&inode->i_mutex);
reiserfs_write_unlock(inode->i_sb);
mutex_unlock(&(REISERFS_I(inode)->tailpack));
return err;
}

static int reiserfs_file_mmap(struct file *file, struct vm_area_struct *vma)
static int reiserfs_file_open(struct inode *inode, struct file *file)
{
struct inode *inode;

inode = file->f_path.dentry->d_inode;
mutex_lock(&(REISERFS_I(inode)->i_mmap));
REISERFS_I(inode)->i_flags |= i_ever_mapped;
mutex_unlock(&(REISERFS_I(inode)->i_mmap));

return generic_file_mmap(file, vma);
int err = dquot_file_open(inode, file);
if (!atomic_inc_not_zero(&REISERFS_I(inode)->openers)) {
/* somebody might be tailpacking on final close; wait for it */
mutex_lock(&(REISERFS_I(inode)->tailpack));
atomic_inc(&REISERFS_I(inode)->openers);
mutex_unlock(&(REISERFS_I(inode)->tailpack));
}
return err;
}

static void reiserfs_vfs_truncate_file(struct inode *inode)
{
mutex_lock(&(REISERFS_I(inode)->tailpack));
reiserfs_truncate_file(inode, 1);
mutex_unlock(&(REISERFS_I(inode)->tailpack));
}

/* Sync a reiserfs file. */
Expand Down Expand Up @@ -288,8 +294,8 @@ const struct file_operations reiserfs_file_operations = {
#ifdef CONFIG_COMPAT
.compat_ioctl = reiserfs_compat_ioctl,
#endif
.mmap = reiserfs_file_mmap,
.open = dquot_file_open,
.mmap = generic_file_mmap,
.open = reiserfs_file_open,
.release = reiserfs_file_release,
.fsync = reiserfs_sync_file,
.aio_read = generic_file_aio_read,
Expand Down
2 changes: 0 additions & 2 deletions fs/reiserfs/inode.c
Original file line number Diff line number Diff line change
Expand Up @@ -1138,7 +1138,6 @@ static void init_inode(struct inode *inode, struct treepath *path)
REISERFS_I(inode)->i_prealloc_count = 0;
REISERFS_I(inode)->i_trans_id = 0;
REISERFS_I(inode)->i_jl = NULL;
mutex_init(&(REISERFS_I(inode)->i_mmap));
reiserfs_init_xattr_rwsem(inode);

if (stat_data_v1(ih)) {
Expand Down Expand Up @@ -1841,7 +1840,6 @@ int reiserfs_new_inode(struct reiserfs_transaction_handle *th,
REISERFS_I(inode)->i_attrs =
REISERFS_I(dir)->i_attrs & REISERFS_INHERIT_MASK;
sd_attrs_to_i_attrs(REISERFS_I(inode)->i_attrs, inode);
mutex_init(&(REISERFS_I(inode)->i_mmap));
reiserfs_init_xattr_rwsem(inode);

/* key to search for correct place for new stat data */
Expand Down
2 changes: 2 additions & 0 deletions fs/reiserfs/super.c
Original file line number Diff line number Diff line change
Expand Up @@ -525,6 +525,8 @@ static struct inode *reiserfs_alloc_inode(struct super_block *sb)
kmem_cache_alloc(reiserfs_inode_cachep, GFP_KERNEL);
if (!ei)
return NULL;
atomic_set(&ei->openers, 0);
mutex_init(&ei->tailpack);
return &ei->vfs_inode;
}

Expand Down
4 changes: 2 additions & 2 deletions include/linux/reiserfs_fs_i.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ typedef enum {
i_link_saved_truncate_mask = 0x0020,
i_has_xattr_dir = 0x0040,
i_data_log = 0x0080,
i_ever_mapped = 0x0100
} reiserfs_inode_flags;

struct reiserfs_inode_info {
Expand Down Expand Up @@ -53,7 +52,8 @@ struct reiserfs_inode_info {
** flushed */
unsigned int i_trans_id;
struct reiserfs_journal_list *i_jl;
struct mutex i_mmap;
atomic_t openers;
struct mutex tailpack;
#ifdef CONFIG_REISERFS_FS_XATTR
struct rw_semaphore i_xattr_sem;
#endif
Expand Down

0 comments on commit 0e4f6a7

Please sign in to comment.