diff mbox

Btrfs: fix a dio write regression

Message ID 1345630274-16242-1-git-send-email-bo.li.liu@oracle.com (mailing list archive)
State New, archived
Headers show

Commit Message

Liu Bo Aug. 22, 2012, 10:11 a.m. UTC 
From: Liu Bo <bo.li.liu@oracle.com>

This bug is introduced by commit 3b8bde746f6f9bd36a9f05f5f3b6e334318176a9
(Btrfs: lock extents as we map them in DIO).

In dio write, we should unlock the section which we didn't do IO on in case that
we fall back to buffered write.  But we need to not only unlock the section
but also cleanup reserved space for the section.

This bug was found while running xfstests 133, with this 133 no longer complains.

Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
---
 fs/btrfs/inode.c |   22 ++++++++++++++++++----
 1 files changed, 18 insertions(+), 4 deletions(-)

Comments

David Sterba Aug. 22, 2012, 3:57 p.m. UTC | #1 
Hi,

a few minor style comments,

On Wed, Aug 22, 2012 at 06:11:14PM +0800, bo.li.liu@oracle.com wrote:
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -5993,10 +5993,24 @@ unlock:
>  	 * in the case of read we need to unlock only the end area that we
>  	 * aren't using if there is any left over space.
>  	 */
> -	if (lockstart < lockend)
> -		clear_extent_bit(&BTRFS_I(inode)->io_tree, lockstart, lockend,
> -				 unlock_bits, 1, 0, &cached_state, GFP_NOFS);
> -	else
> +	if (lockstart < lockend) {
> +		if (create && len < lockend - lockstart) {
> +			clear_extent_bit(&BTRFS_I(inode)->io_tree, lockstart,
> +					 lockstart + len - 1, unlock_bits, 1, 0,
> +					 &cached_state, GFP_NOFS);
> +			/*
> +			 * Beside unlock, we also need to cleanup reserved space
> +			 * for the left range by attaching EXTENT_DO_ACCOUNTING.
> +			 */
> +			clear_extent_bit(&BTRFS_I(inode)->io_tree,
> +					 lockstart + len, lockend, unlock_bits |
> +					 EXTENT_DO_ACCOUNTING, 1, 0, NULL,

I'd prefer to see unlock_bits and the new value on one line

> +					 GFP_NOFS);
> +		} else

add { ... } around this

> +			clear_extent_bit(&BTRFS_I(inode)->io_tree, lockstart,
> +					 lockend, unlock_bits, 1, 0,
> +					 &cached_state, GFP_NOFS);
> +	} else

here too

>  		free_extent_state(cached_state);
>  
>  	free_extent_map(em);
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 7131fac..e4ab92b 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -5993,10 +5993,24 @@  unlock:
 	 * in the case of read we need to unlock only the end area that we
 	 * aren't using if there is any left over space.
 	 */
-	if (lockstart < lockend)
-		clear_extent_bit(&BTRFS_I(inode)->io_tree, lockstart, lockend,
-				 unlock_bits, 1, 0, &cached_state, GFP_NOFS);
-	else
+	if (lockstart < lockend) {
+		if (create && len < lockend - lockstart) {
+			clear_extent_bit(&BTRFS_I(inode)->io_tree, lockstart,
+					 lockstart + len - 1, unlock_bits, 1, 0,
+					 &cached_state, GFP_NOFS);
+			/*
+			 * Beside unlock, we also need to cleanup reserved space
+			 * for the left range by attaching EXTENT_DO_ACCOUNTING.
+			 */
+			clear_extent_bit(&BTRFS_I(inode)->io_tree,
+					 lockstart + len, lockend, unlock_bits |
+					 EXTENT_DO_ACCOUNTING, 1, 0, NULL,
+					 GFP_NOFS);
+		} else
+			clear_extent_bit(&BTRFS_I(inode)->io_tree, lockstart,
+					 lockend, unlock_bits, 1, 0,
+					 &cached_state, GFP_NOFS);
+	} else
 		free_extent_state(cached_state);
 
 	free_extent_map(em);