diff mbox

[1/2] Btrfs: fix fsync after hole punching when using no-holes feature

Message ID 20180326225900.9979-1-fdmanana@kernel.org (mailing list archive)
State New, archived
Headers show

Commit Message

Filipe Manana March 26, 2018, 10:59 p.m. UTC
From: Filipe Manana <fdmanana@suse.com>

When we have the no-holes mode enabled and fsync a file after punching a
hole in it, we can end up not logging the whole hole range in the log tree.
This happens if the file has extent items that span more than one leaf and
we punch a hole that covers a range that starts in a leaf but does not go
beyond the offset of the first extent in the next leaf.

Example:

  $ mkfs.btrfs -f -O no-holes -n 65536 /dev/sdb
  $ mount /dev/sdb /mnt
  $ for ((i = 0; i <= 831; i++)); do
	offset=$((i * 2 * 256 * 1024))
	xfs_io -f -c "pwrite -S 0xab -b 256K $offset 256K" \
		/mnt/foobar >/dev/null
    done
  $ sync

  # We now have 2 leafs in our filesystem fs tree, the first leaf has an
  # item corresponding the extent at file offset 216530944 and the second
  # leaf has a first item corresponding to the extent at offset 217055232.
  # Now we punch a hole that partially covers the range of the extent at
  # offset 216530944 but does go beyond the offset 217055232.

  $ xfs_io -c "fpunch $((216530944 + 128 * 1024 - 4000)) 256K" /mnt/foobar
  $ xfs_io -c "fsync" /mnt/foobar

  <power fail>

  # mount to replay the log
  $ mount /dev/sdb /mnt

  # Before this patch, only the subrange [216658016, 216662016[ (length of
  # 4000 bytes) was logged, leaving an incorrect file layout after log
  # replay.

Fix this by checking if there is a hole between the last extent item that
we processed and the first extent item in the next leaf, and if there is
one, log an explicit hole extent item.

Fixes: 16e7549f045d ("Btrfs: incompatible format change to remove hole extents")
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
 fs/btrfs/tree-log.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

Comments

David Sterba March 28, 2018, 2:37 p.m. UTC | #1
On Mon, Mar 26, 2018 at 11:59:00PM +0100, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> When we have the no-holes mode enabled and fsync a file after punching a
> hole in it, we can end up not logging the whole hole range in the log tree.
> This happens if the file has extent items that span more than one leaf and
> we punch a hole that covers a range that starts in a leaf but does not go
> beyond the offset of the first extent in the next leaf.
> 
> Example:
> 
>   $ mkfs.btrfs -f -O no-holes -n 65536 /dev/sdb
>   $ mount /dev/sdb /mnt
>   $ for ((i = 0; i <= 831; i++)); do
> 	offset=$((i * 2 * 256 * 1024))
> 	xfs_io -f -c "pwrite -S 0xab -b 256K $offset 256K" \
> 		/mnt/foobar >/dev/null
>     done
>   $ sync
> 
>   # We now have 2 leafs in our filesystem fs tree, the first leaf has an
>   # item corresponding the extent at file offset 216530944 and the second
>   # leaf has a first item corresponding to the extent at offset 217055232.
>   # Now we punch a hole that partially covers the range of the extent at
>   # offset 216530944 but does go beyond the offset 217055232.
> 
>   $ xfs_io -c "fpunch $((216530944 + 128 * 1024 - 4000)) 256K" /mnt/foobar
>   $ xfs_io -c "fsync" /mnt/foobar
> 
>   <power fail>
> 
>   # mount to replay the log
>   $ mount /dev/sdb /mnt
> 
>   # Before this patch, only the subrange [216658016, 216662016[ (length of
>   # 4000 bytes) was logged, leaving an incorrect file layout after log
>   # replay.
> 
> Fix this by checking if there is a hole between the last extent item that
> we processed and the first extent item in the next leaf, and if there is
> one, log an explicit hole extent item.
> 
> Fixes: 16e7549f045d ("Btrfs: incompatible format change to remove hole extents")
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

1 and 2 added to next, thanks.
--
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/tree-log.c b/fs/btrfs/tree-log.c
index fd573816f461..1d738ff5c41b 100644
--- a/fs/btrfs/tree-log.c
+++ b/fs/btrfs/tree-log.c
@@ -3994,6 +3994,36 @@  static noinline int copy_items(struct btrfs_trans_handle *trans,
 			break;
 		*last_extent = extent_end;
 	}
+
+	/*
+	 * Check if there is a hole between the last extent found in our leaf
+	 * and the first extent in the next leaf. If there is one, we need to
+	 * log an explicit hole so that at replay time we can punch the hole.
+	 */
+	if (ret == 0 &&
+	    key.objectid == btrfs_ino(inode) &&
+	    key.type == BTRFS_EXTENT_DATA_KEY &&
+	    i == btrfs_header_nritems(src_path->nodes[0])) {
+		ret = btrfs_next_leaf(inode->root, src_path);
+		need_find_last_extent = true;
+		if (ret > 0) {
+			ret = 0;
+		} else if (ret == 0) {
+			btrfs_item_key_to_cpu(src_path->nodes[0], &key,
+					      src_path->slots[0]);
+			if (key.objectid == btrfs_ino(inode) &&
+			    key.type == BTRFS_EXTENT_DATA_KEY &&
+			    *last_extent < key.offset) {
+				const u64 len = key.offset - *last_extent;
+
+				ret = btrfs_insert_file_extent(trans, log,
+							       btrfs_ino(inode),
+							       *last_extent, 0,
+							       0, len, 0, len,
+							       0, 0, 0);
+			}
+		}
+	}
 	/*
 	 * Need to let the callers know we dropped the path so they should
 	 * re-search.