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/stree.c | 46 +++++++++++++++++++++------------------------- 1 file changed, 21 insertions(+), 25 deletions(-) (limited to 'fs/reiserfs/stree.c') diff --git a/fs/reiserfs/stree.c b/fs/reiserfs/stree.c index 2f40a4c70a4d..4d7d476d7bff 100644 --- a/fs/reiserfs/stree.c +++ b/fs/reiserfs/stree.c @@ -524,14 +524,14 @@ static int is_tree_node(struct buffer_head *bh, int level) * the caller (search_by_key) will perform other schedule-unsafe * operations just after calling this function. * - * @return true if we have unlocked + * @return depth of lock to be restored after read completes */ -static bool search_by_key_reada(struct super_block *s, +static int search_by_key_reada(struct super_block *s, struct buffer_head **bh, b_blocknr_t *b, int num) { int i, j; - bool unlocked = false; + int depth = -1; for (i = 0; i < num; i++) { bh[i] = sb_getblk(s, b[i]); @@ -549,15 +549,13 @@ static bool search_by_key_reada(struct super_block *s, * you have to make sure the prepared bit isn't set on this buffer */ if (!buffer_uptodate(bh[j])) { - if (!unlocked) { - reiserfs_write_unlock(s); - unlocked = true; - } + if (depth == -1) + depth = reiserfs_write_unlock_nested(s); ll_rw_block(READA, 1, bh + j); } brelse(bh[j]); } - return unlocked; + return depth; } /************************************************************************** @@ -645,26 +643,26 @@ int search_by_key(struct super_block *sb, const struct cpu_key *key, /* Key to s have a pointer to it. */ if ((bh = last_element->pe_buffer = sb_getblk(sb, block_number))) { - bool unlocked = false; - if (!buffer_uptodate(bh) && reada_count > 1) - /* may unlock the write lock */ - unlocked = search_by_key_reada(sb, reada_bh, - reada_blocks, reada_count); /* - * If we haven't already unlocked the write lock, - * then we need to do that here before reading - * the current block + * We'll need to drop the lock if we encounter any + * buffers that need to be read. If all of them are + * already up to date, we don't need to drop the lock. */ - if (!buffer_uptodate(bh) && !unlocked) { - reiserfs_write_unlock(sb); - unlocked = true; - } + int depth = -1; + + if (!buffer_uptodate(bh) && reada_count > 1) + depth = search_by_key_reada(sb, reada_bh, + reada_blocks, reada_count); + + if (!buffer_uptodate(bh) && depth == -1) + depth = reiserfs_write_unlock_nested(sb); + ll_rw_block(READ, 1, &bh); wait_on_buffer(bh); - if (unlocked) - reiserfs_write_lock(sb); + if (depth != -1) + reiserfs_write_lock_nested(sb, depth); if (!buffer_uptodate(bh)) goto io_error; } else { @@ -1059,9 +1057,7 @@ static char prepare_for_delete_or_cut(struct reiserfs_transaction_handle *th, st reiserfs_free_block(th, inode, block, 1); } - reiserfs_write_unlock(sb); - cond_resched(); - reiserfs_write_lock(sb); + reiserfs_cond_resched(sb); if (item_moved (&s_ih, path)) { need_re_search = 1; -- cgit v1.2.3 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/stree.c | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) (limited to 'fs/reiserfs/stree.c') diff --git a/fs/reiserfs/stree.c b/fs/reiserfs/stree.c index 4d7d476d7bff..b14706a05d52 100644 --- a/fs/reiserfs/stree.c +++ b/fs/reiserfs/stree.c @@ -1186,6 +1186,7 @@ int reiserfs_delete_item(struct reiserfs_transaction_handle *th, struct item_head *q_ih; int quota_cut_bytes; int ret_value, del_size, removed; + int depth; #ifdef CONFIG_REISERFS_CHECK char mode; @@ -1295,7 +1296,9 @@ int reiserfs_delete_item(struct reiserfs_transaction_handle *th, "reiserquota delete_item(): freeing %u, id=%u type=%c", quota_cut_bytes, inode->i_uid, head2type(&s_ih)); #endif + depth = reiserfs_write_unlock_nested(inode->i_sb); dquot_free_space_nodirty(inode, quota_cut_bytes); + reiserfs_write_lock_nested(inode->i_sb, depth); /* Return deleted body length */ return ret_value; @@ -1321,6 +1324,7 @@ int reiserfs_delete_item(struct reiserfs_transaction_handle *th, void reiserfs_delete_solid_item(struct reiserfs_transaction_handle *th, struct inode *inode, struct reiserfs_key *key) { + struct super_block *sb = th->t_super; struct tree_balance tb; INITIALIZE_PATH(path); int item_len = 0; @@ -1373,14 +1377,17 @@ void reiserfs_delete_solid_item(struct reiserfs_transaction_handle *th, if (retval == CARRY_ON) { do_balance(&tb, NULL, NULL, M_DELETE); if (inode) { /* Should we count quota for item? (we don't count quotas for save-links) */ + int depth; #ifdef REISERQUOTA_DEBUG reiserfs_debug(th->t_super, REISERFS_DEBUG_CODE, "reiserquota delete_solid_item(): freeing %u id=%u type=%c", quota_cut_bytes, inode->i_uid, key2type(key)); #endif + depth = reiserfs_write_unlock_nested(sb); dquot_free_space_nodirty(inode, quota_cut_bytes); + reiserfs_write_lock_nested(sb, depth); } break; } @@ -1557,6 +1564,7 @@ int reiserfs_cut_from_item(struct reiserfs_transaction_handle *th, int retval2 = -1; int quota_cut_bytes; loff_t tail_pos = 0; + int depth; BUG_ON(!th->t_trans_id); @@ -1729,7 +1737,9 @@ int reiserfs_cut_from_item(struct reiserfs_transaction_handle *th, "reiserquota cut_from_item(): freeing %u id=%u type=%c", quota_cut_bytes, inode->i_uid, '?'); #endif + depth = reiserfs_write_unlock_nested(sb); dquot_free_space_nodirty(inode, quota_cut_bytes); + reiserfs_write_lock_nested(sb, depth); return ret_value; } @@ -1949,9 +1959,11 @@ int reiserfs_paste_into_item(struct reiserfs_transaction_handle *th, struct tree const char *body, /* Pointer to the bytes to paste. */ int pasted_size) { /* Size of pasted bytes. */ + struct super_block *sb = inode->i_sb; struct tree_balance s_paste_balance; int retval; int fs_gen; + int depth; BUG_ON(!th->t_trans_id); @@ -1964,9 +1976,9 @@ int reiserfs_paste_into_item(struct reiserfs_transaction_handle *th, struct tree key2type(&(key->on_disk_key))); #endif - reiserfs_write_unlock(inode->i_sb); + depth = reiserfs_write_unlock_nested(sb); retval = dquot_alloc_space_nodirty(inode, pasted_size); - reiserfs_write_lock(inode->i_sb); + reiserfs_write_lock_nested(sb, depth); if (retval) { pathrelse(search_path); return retval; @@ -2023,7 +2035,9 @@ int reiserfs_paste_into_item(struct reiserfs_transaction_handle *th, struct tree pasted_size, inode->i_uid, key2type(&(key->on_disk_key))); #endif + depth = reiserfs_write_unlock_nested(sb); dquot_free_space_nodirty(inode, pasted_size); + reiserfs_write_lock_nested(sb, depth); return retval; } @@ -2046,6 +2060,7 @@ int reiserfs_insert_item(struct reiserfs_transaction_handle *th, BUG_ON(!th->t_trans_id); if (inode) { /* Do we count quotas for item? */ + int depth; fs_gen = get_generation(inode->i_sb); quota_bytes = ih_item_len(ih); @@ -2059,11 +2074,11 @@ int reiserfs_insert_item(struct reiserfs_transaction_handle *th, "reiserquota insert_item(): allocating %u id=%u type=%c", quota_bytes, inode->i_uid, head2type(ih)); #endif - reiserfs_write_unlock(inode->i_sb); /* We can't dirty inode here. It would be immediately written but * appropriate stat item isn't inserted yet... */ + depth = reiserfs_write_unlock_nested(inode->i_sb); retval = dquot_alloc_space_nodirty(inode, quota_bytes); - reiserfs_write_lock(inode->i_sb); + reiserfs_write_lock_nested(inode->i_sb, depth); if (retval) { pathrelse(path); return retval; @@ -2114,7 +2129,10 @@ int reiserfs_insert_item(struct reiserfs_transaction_handle *th, "reiserquota insert_item(): freeing %u id=%u type=%c", quota_bytes, inode->i_uid, head2type(ih)); #endif - if (inode) + if (inode) { + int depth = reiserfs_write_unlock_nested(inode->i_sb); dquot_free_space_nodirty(inode, quota_bytes); + reiserfs_write_lock_nested(inode->i_sb, depth); + } return retval; } -- cgit v1.2.3