From 4c05141df57f4ffc1a9a28f1925434924179bfe4 Mon Sep 17 00:00:00 2001 From: Jeff Mahoney Date: Thu, 8 Aug 2013 17:27:19 -0400 Subject: reiserfs: locking, push write lock out of xattr code The reiserfs xattr code doesn't need the write lock and sleeps all over the place. We can simplify the locking by releasing it and reacquiring after the xattr call. Signed-off-by: Jeff Mahoney --- fs/reiserfs/super.c | 48 +++++++++++++++++++++++------------------------- 1 file changed, 23 insertions(+), 25 deletions(-) (limited to 'fs/reiserfs/super.c') diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c index f8a23c3078f8..7e81d9774916 100644 --- a/fs/reiserfs/super.c +++ b/fs/reiserfs/super.c @@ -1335,7 +1335,7 @@ static int reiserfs_remount(struct super_block *s, int *mount_flags, char *arg) kfree(qf_names[i]); #endif err = -EINVAL; - goto out_unlock; + goto out_err_unlock; } #ifdef CONFIG_QUOTA handle_quota_files(s, qf_names, &qfmt); @@ -1379,35 +1379,32 @@ static int reiserfs_remount(struct super_block *s, int *mount_flags, char *arg) if (blocks) { err = reiserfs_resize(s, blocks); if (err != 0) - goto out_unlock; + goto out_err_unlock; } if (*mount_flags & MS_RDONLY) { + reiserfs_write_unlock(s); reiserfs_xattr_init(s, *mount_flags); /* remount read-only */ if (s->s_flags & MS_RDONLY) /* it is read-only already */ - goto out_ok; + goto out_ok_unlocked; - /* - * Drop write lock. Quota will retake it when needed and lock - * ordering requires calling dquot_suspend() without it. - */ - reiserfs_write_unlock(s); err = dquot_suspend(s, -1); if (err < 0) goto out_err; - reiserfs_write_lock(s); /* try to remount file system with read-only permissions */ if (sb_umount_state(rs) == REISERFS_VALID_FS || REISERFS_SB(s)->s_mount_state != REISERFS_VALID_FS) { - goto out_ok; + goto out_ok_unlocked; } + reiserfs_write_lock(s); + err = journal_begin(&th, s, 10); if (err) - goto out_unlock; + goto out_err_unlock; /* Mounting a rw partition read-only. */ reiserfs_prepare_for_journal(s, SB_BUFFER_WITH_SB(s), 1); @@ -1416,13 +1413,14 @@ static int reiserfs_remount(struct super_block *s, int *mount_flags, char *arg) } else { /* remount read-write */ if (!(s->s_flags & MS_RDONLY)) { + reiserfs_write_unlock(s); reiserfs_xattr_init(s, *mount_flags); - goto out_ok; /* We are read-write already */ + goto out_ok_unlocked; /* We are read-write already */ } if (reiserfs_is_journal_aborted(journal)) { err = journal->j_errno; - goto out_unlock; + goto out_err_unlock; } handle_data_mode(s, mount_options); @@ -1431,7 +1429,7 @@ static int reiserfs_remount(struct super_block *s, int *mount_flags, char *arg) s->s_flags &= ~MS_RDONLY; /* now it is safe to call journal_begin */ err = journal_begin(&th, s, 10); if (err) - goto out_unlock; + goto out_err_unlock; /* Mount a partition which is read-only, read-write */ reiserfs_prepare_for_journal(s, SB_BUFFER_WITH_SB(s), 1); @@ -1448,26 +1446,22 @@ static int reiserfs_remount(struct super_block *s, int *mount_flags, char *arg) SB_JOURNAL(s)->j_must_wait = 1; err = journal_end(&th, s, 10); if (err) - goto out_unlock; + goto out_err_unlock; + reiserfs_write_unlock(s); if (!(*mount_flags & MS_RDONLY)) { - /* - * Drop write lock. Quota will retake it when needed and lock - * ordering requires calling dquot_resume() without it. - */ - reiserfs_write_unlock(s); dquot_resume(s, -1); reiserfs_write_lock(s); finish_unfinished(s); + reiserfs_write_unlock(s); reiserfs_xattr_init(s, *mount_flags); } -out_ok: +out_ok_unlocked: replace_mount_options(s, new_opts); - reiserfs_write_unlock(s); return 0; -out_unlock: +out_err_unlock: reiserfs_write_unlock(s); out_err: kfree(new_opts); @@ -2014,12 +2008,14 @@ static int reiserfs_fill_super(struct super_block *s, void *data, int silent) goto error; } + reiserfs_write_unlock(s); if ((errval = reiserfs_lookup_privroot(s)) || (errval = reiserfs_xattr_init(s, s->s_flags))) { dput(s->s_root); s->s_root = NULL; - goto error; + goto error_unlocked; } + reiserfs_write_lock(s); /* look for files which were to be removed in previous session */ finish_unfinished(s); @@ -2028,12 +2024,14 @@ static int reiserfs_fill_super(struct super_block *s, void *data, int silent) reiserfs_info(s, "using 3.5.x disk format\n"); } + reiserfs_write_unlock(s); if ((errval = reiserfs_lookup_privroot(s)) || (errval = reiserfs_xattr_init(s, s->s_flags))) { dput(s->s_root); s->s_root = NULL; - goto error; + goto error_unlocked; } + reiserfs_write_lock(s); } // mark hash in super block: it could be unset. overwrite should be ok set_sb_hash_function_code(rs, function2code(sbi->s_hash_function)); -- cgit v1.2.2 From 278f6679f454bf185a07d9a4ca355b153482d17a Mon Sep 17 00:00:00 2001 From: Jeff Mahoney Date: Thu, 8 Aug 2013 17:34:46 -0400 Subject: reiserfs: locking, handle nested locks properly The reiserfs write lock replaced the BKL and uses similar semantics. Frederic's locking code makes a distinction between when the lock is nested and when it's being acquired/released, but I don't think that's the right distinction to make. The right distinction is between the lock being released at end-of-use and the lock being released for a schedule. The unlock should return the depth and the lock should restore it, rather than the other way around as it is now. This patch implements that and adds a number of places where the lock should be dropped. Signed-off-by: Jeff Mahoney --- fs/reiserfs/super.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'fs/reiserfs/super.c') diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c index 7e81d9774916..60d09327e587 100644 --- a/fs/reiserfs/super.c +++ b/fs/reiserfs/super.c @@ -624,7 +624,6 @@ static void reiserfs_dirty_inode(struct inode *inode, int flags) struct reiserfs_transaction_handle th; int err = 0; - int lock_depth; if (inode->i_sb->s_flags & MS_RDONLY) { reiserfs_warning(inode->i_sb, "clm-6006", @@ -632,7 +631,7 @@ static void reiserfs_dirty_inode(struct inode *inode, int flags) inode->i_ino); return; } - lock_depth = reiserfs_write_lock_once(inode->i_sb); + reiserfs_write_lock(inode->i_sb); /* this is really only used for atime updates, so they don't have ** to be included in O_SYNC or fsync @@ -645,7 +644,7 @@ static void reiserfs_dirty_inode(struct inode *inode, int flags) journal_end(&th, inode->i_sb, 1); out: - reiserfs_write_unlock_once(inode->i_sb, lock_depth); + reiserfs_write_unlock(inode->i_sb); } static int reiserfs_show_options(struct seq_file *seq, struct dentry *root) -- cgit v1.2.2 From d2d0395fd1778d4bf714adc5bfd23a5d748d7802 Mon Sep 17 00:00:00 2001 From: Jeff Mahoney Date: Thu, 8 Aug 2013 17:34:47 -0400 Subject: reiserfs: locking, release lock around quota operations Previous commits released the write lock across quota operations but missed several places. In particular, the free operations can also call into the file system code and take the write lock, causing deadlocks. This patch introduces some more helpers and uses them for quota call sites. Without this patch applied, reiserfs + quotas runs into deadlocks under anything more than trivial load. Signed-off-by: Jeff Mahoney --- fs/reiserfs/super.c | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) (limited to 'fs/reiserfs/super.c') diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c index 60d09327e587..bb20d791bbdc 100644 --- a/fs/reiserfs/super.c +++ b/fs/reiserfs/super.c @@ -243,6 +243,7 @@ static int finish_unfinished(struct super_block *s) done = 0; REISERFS_SB(s)->s_is_unlinked_ok = 1; while (!retval) { + int depth; retval = search_item(s, &max_cpu_key, &path); if (retval != ITEM_NOT_FOUND) { reiserfs_error(s, "vs-2140", @@ -298,9 +299,9 @@ static int finish_unfinished(struct super_block *s) retval = remove_save_link_only(s, &save_link_key, 0); continue; } - reiserfs_write_unlock(s); + depth = reiserfs_write_unlock_nested(inode->i_sb); dquot_initialize(inode); - reiserfs_write_lock(s); + reiserfs_write_lock_nested(inode->i_sb, depth); if (truncate && S_ISDIR(inode->i_mode)) { /* We got a truncate request for a dir which is impossible. @@ -356,10 +357,12 @@ static int finish_unfinished(struct super_block *s) #ifdef CONFIG_QUOTA /* Turn quotas off */ + reiserfs_write_unlock(s); for (i = 0; i < MAXQUOTAS; i++) { if (sb_dqopt(s)->files[i] && quota_enabled[i]) dquot_quota_off(s, i); } + reiserfs_write_lock(s); if (ms_active_set) /* Restore the flag back */ s->s_flags &= ~MS_ACTIVE; @@ -2098,6 +2101,7 @@ static int reiserfs_write_dquot(struct dquot *dquot) { struct reiserfs_transaction_handle th; int ret, err; + int depth; reiserfs_write_lock(dquot->dq_sb); ret = @@ -2105,9 +2109,9 @@ static int reiserfs_write_dquot(struct dquot *dquot) REISERFS_QUOTA_TRANS_BLOCKS(dquot->dq_sb)); if (ret) goto out; - reiserfs_write_unlock(dquot->dq_sb); + depth = reiserfs_write_unlock_nested(dquot->dq_sb); ret = dquot_commit(dquot); - reiserfs_write_lock(dquot->dq_sb); + reiserfs_write_lock_nested(dquot->dq_sb, depth); err = journal_end(&th, dquot->dq_sb, REISERFS_QUOTA_TRANS_BLOCKS(dquot->dq_sb)); @@ -2122,6 +2126,7 @@ static int reiserfs_acquire_dquot(struct dquot *dquot) { struct reiserfs_transaction_handle th; int ret, err; + int depth; reiserfs_write_lock(dquot->dq_sb); ret = @@ -2129,9 +2134,9 @@ static int reiserfs_acquire_dquot(struct dquot *dquot) REISERFS_QUOTA_INIT_BLOCKS(dquot->dq_sb)); if (ret) goto out; - reiserfs_write_unlock(dquot->dq_sb); + depth = reiserfs_write_unlock_nested(dquot->dq_sb); ret = dquot_acquire(dquot); - reiserfs_write_lock(dquot->dq_sb); + reiserfs_write_lock_nested(dquot->dq_sb, depth); err = journal_end(&th, dquot->dq_sb, REISERFS_QUOTA_INIT_BLOCKS(dquot->dq_sb)); @@ -2184,15 +2189,16 @@ static int reiserfs_write_info(struct super_block *sb, int type) { struct reiserfs_transaction_handle th; int ret, err; + int depth; /* Data block + inode block */ reiserfs_write_lock(sb); ret = journal_begin(&th, sb, 2); if (ret) goto out; - reiserfs_write_unlock(sb); + depth = reiserfs_write_unlock_nested(sb); ret = dquot_commit_info(sb, type); - reiserfs_write_lock(sb); + reiserfs_write_lock_nested(sb, depth); err = journal_end(&th, sb, 2); if (!ret && err) ret = err; -- cgit v1.2.2