diff options
author | Frederic Weisbecker <fweisbec@gmail.com> | 2009-05-07 17:48:44 -0400 |
---|---|---|
committer | Frederic Weisbecker <fweisbec@gmail.com> | 2009-09-14 01:18:16 -0400 |
commit | 26931309a47747fd31b2ef029c29d47794c2d93d (patch) | |
tree | 85b3ba9ee14c6d90243e4590af60e7ef3e72c5ff /fs | |
parent | d663af807d8bb226394cb7e02f4665f6141a8140 (diff) |
kill-the-bkl/reiserfs: lock only once on reiserfs_get_block()
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>
Diffstat (limited to 'fs')
-rw-r--r-- | fs/reiserfs/inode.c | 19 |
1 files changed, 11 insertions, 8 deletions
diff --git a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c index cc70b56bf6f2..6114050f342e 100644 --- a/fs/reiserfs/inode.c +++ b/fs/reiserfs/inode.c | |||
@@ -605,6 +605,7 @@ int reiserfs_get_block(struct inode *inode, sector_t block, | |||
605 | __le32 *item; | 605 | __le32 *item; |
606 | int done; | 606 | int done; |
607 | int fs_gen; | 607 | int fs_gen; |
608 | int lock_depth; | ||
608 | struct reiserfs_transaction_handle *th = NULL; | 609 | struct reiserfs_transaction_handle *th = NULL; |
609 | /* space reserved in transaction batch: | 610 | /* space reserved in transaction batch: |
610 | . 3 balancings in direct->indirect conversion | 611 | . 3 balancings in direct->indirect conversion |
@@ -620,11 +621,11 @@ int reiserfs_get_block(struct inode *inode, sector_t block, | |||
620 | loff_t new_offset = | 621 | loff_t new_offset = |
621 | (((loff_t) block) << inode->i_sb->s_blocksize_bits) + 1; | 622 | (((loff_t) block) << inode->i_sb->s_blocksize_bits) + 1; |
622 | 623 | ||
623 | reiserfs_write_lock(inode->i_sb); | 624 | lock_depth = reiserfs_write_lock_once(inode->i_sb); |
624 | version = get_inode_item_key_version(inode); | 625 | version = get_inode_item_key_version(inode); |
625 | 626 | ||
626 | if (!file_capable(inode, block)) { | 627 | if (!file_capable(inode, block)) { |
627 | reiserfs_write_unlock(inode->i_sb); | 628 | reiserfs_write_unlock_once(inode->i_sb, lock_depth); |
628 | return -EFBIG; | 629 | return -EFBIG; |
629 | } | 630 | } |
630 | 631 | ||
@@ -636,7 +637,7 @@ int reiserfs_get_block(struct inode *inode, sector_t block, | |||
636 | /* find number of block-th logical block of the file */ | 637 | /* find number of block-th logical block of the file */ |
637 | ret = _get_block_create_0(inode, block, bh_result, | 638 | ret = _get_block_create_0(inode, block, bh_result, |
638 | create | GET_BLOCK_READ_DIRECT); | 639 | create | GET_BLOCK_READ_DIRECT); |
639 | reiserfs_write_unlock(inode->i_sb); | 640 | reiserfs_write_unlock_once(inode->i_sb, lock_depth); |
640 | return ret; | 641 | return ret; |
641 | } | 642 | } |
642 | /* | 643 | /* |
@@ -754,7 +755,7 @@ int reiserfs_get_block(struct inode *inode, sector_t block, | |||
754 | if (!dangle && th) | 755 | if (!dangle && th) |
755 | retval = reiserfs_end_persistent_transaction(th); | 756 | retval = reiserfs_end_persistent_transaction(th); |
756 | 757 | ||
757 | reiserfs_write_unlock(inode->i_sb); | 758 | reiserfs_write_unlock_once(inode->i_sb, lock_depth); |
758 | 759 | ||
759 | /* the item was found, so new blocks were not added to the file | 760 | /* the item was found, so new blocks were not added to the file |
760 | ** there is no need to make sure the inode is updated with this | 761 | ** 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, | |||
1005 | * long time. reschedule if needed and also release the write | 1006 | * long time. reschedule if needed and also release the write |
1006 | * lock for others. | 1007 | * lock for others. |
1007 | */ | 1008 | */ |
1008 | reiserfs_write_unlock(inode->i_sb); | 1009 | if (need_resched()) { |
1009 | cond_resched(); | 1010 | reiserfs_write_unlock_once(inode->i_sb, lock_depth); |
1010 | reiserfs_write_lock(inode->i_sb); | 1011 | schedule(); |
1012 | lock_depth = reiserfs_write_lock_once(inode->i_sb); | ||
1013 | } | ||
1011 | 1014 | ||
1012 | retval = search_for_position_by_key(inode->i_sb, &key, &path); | 1015 | retval = search_for_position_by_key(inode->i_sb, &key, &path); |
1013 | if (retval == IO_ERROR) { | 1016 | if (retval == IO_ERROR) { |
@@ -1042,7 +1045,7 @@ int reiserfs_get_block(struct inode *inode, sector_t block, | |||
1042 | retval = err; | 1045 | retval = err; |
1043 | } | 1046 | } |
1044 | 1047 | ||
1045 | reiserfs_write_unlock(inode->i_sb); | 1048 | reiserfs_write_unlock_once(inode->i_sb, lock_depth); |
1046 | reiserfs_check_path(&path); | 1049 | reiserfs_check_path(&path); |
1047 | return retval; | 1050 | return retval; |
1048 | } | 1051 | } |