kill-the-bkl/reiserfs: lock only once on reiserfs_get_block()
authorFrederic Weisbecker <fweisbec@gmail.com>
Thu, 7 May 2009 21:48:44 +0000 (23:48 +0200)
committerFrederic Weisbecker <fweisbec@gmail.com>
Mon, 14 Sep 2009 05:18:16 +0000 (07:18 +0200)
reiserfs_get_block() is one of these sites where the write lock might
be acquired recursively.

It's a particular problem because this function is called very often.
It's a hot spot which needs to reschedule() periodically while converting
direct items to indirect ones because it can take some time.

Then if we are applying the write lock release/reacquire pattern on
schedule() here, it may not produce the desired effect since we may have
locked in more than one depth.

The solution is to use reiserfs_write_lock_once() which won't try
to reacquire the lock recursively. Then the lock will be *really*
released before schedule().

Also, we only release the lock if TIF_NEED_RESCHED is set to not
create wasteful numerous contentions.

[ Impact: fix a too long holded lock case in reiserfs_get_block() ]

Cc: Jeff Mahoney <jeffm@suse.com>
Cc: Chris Mason <chris.mason@oracle.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Alexander Beregalov <a.beregalov@gmail.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
fs/reiserfs/inode.c

index cc70b56..6114050 100644 (file)
@@ -605,6 +605,7 @@ int reiserfs_get_block(struct inode *inode, sector_t block,
        __le32 *item;
        int done;
        int fs_gen;
+       int lock_depth;
        struct reiserfs_transaction_handle *th = NULL;
        /* space reserved in transaction batch:
           . 3 balancings in direct->indirect conversion
@@ -620,11 +621,11 @@ int reiserfs_get_block(struct inode *inode, sector_t block,
        loff_t new_offset =
            (((loff_t) block) << inode->i_sb->s_blocksize_bits) + 1;
 
-       reiserfs_write_lock(inode->i_sb);
+       lock_depth = reiserfs_write_lock_once(inode->i_sb);
        version = get_inode_item_key_version(inode);
 
        if (!file_capable(inode, block)) {
-               reiserfs_write_unlock(inode->i_sb);
+               reiserfs_write_unlock_once(inode->i_sb, lock_depth);
                return -EFBIG;
        }
 
@@ -636,7 +637,7 @@ int reiserfs_get_block(struct inode *inode, sector_t block,
                /* find number of block-th logical block of the file */
                ret = _get_block_create_0(inode, block, bh_result,
                                          create | GET_BLOCK_READ_DIRECT);
-               reiserfs_write_unlock(inode->i_sb);
+               reiserfs_write_unlock_once(inode->i_sb, lock_depth);
                return ret;
        }
        /*
@@ -754,7 +755,7 @@ int reiserfs_get_block(struct inode *inode, sector_t block,
                if (!dangle && th)
                        retval = reiserfs_end_persistent_transaction(th);
 
-               reiserfs_write_unlock(inode->i_sb);
+               reiserfs_write_unlock_once(inode->i_sb, lock_depth);
 
                /* the item was found, so new blocks were not added to the file
                 ** there is no need to make sure the inode is updated with this
@@ -1005,9 +1006,11 @@ int reiserfs_get_block(struct inode *inode, sector_t block,
                 * long time.  reschedule if needed and also release the write
                 * lock for others.
                 */
-               reiserfs_write_unlock(inode->i_sb);
-               cond_resched();
-               reiserfs_write_lock(inode->i_sb);
+               if (need_resched()) {
+                       reiserfs_write_unlock_once(inode->i_sb, lock_depth);
+                       schedule();
+                       lock_depth = reiserfs_write_lock_once(inode->i_sb);
+               }
 
                retval = search_for_position_by_key(inode->i_sb, &key, &path);
                if (retval == IO_ERROR) {
@@ -1042,7 +1045,7 @@ int reiserfs_get_block(struct inode *inode, sector_t block,
                        retval = err;
        }
 
-       reiserfs_write_unlock(inode->i_sb);
+       reiserfs_write_unlock_once(inode->i_sb, lock_depth);
        reiserfs_check_path(&path);
        return retval;
 }