Filipe noticed a race where we would sometimes get the wrong answer for
the i_disk_size for !NO_HOLES with my patch. That is because I expected
that find_first_extent_bit() would find the contiguous range, since I'm
only ever setting EXTENT_DIRTY. However the way set_extent_bit() works
is it'll temporarily split the range, loop around and set our bits, and
then merge the state. When it loops it drops the tree->lock, so there
is a window where we can have two adjacent states instead of one large
state. Fix this by walking forward until we find a non-contiguous
state, and set our end_ret to the end of our logically contiguous area.
This fixes the problem without relying on specific behavior from
set_extent_bit().
Fixes: 79ceff7f6e5d ("btrfs: introduce per-inode file extent tree")
Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
---
Dave, I assume you'll want to fold this in to the referenced patch, if not let
me know and I'll rework the series to include this as a different patch.
fs/btrfs/extent-io-tree.h | 2 ++
fs/btrfs/extent_io.c | 36 ++++++++++++++++++++++++++++++++++++
fs/btrfs/file-item.c | 4 ++--
3 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/extent-io-tree.h b/fs/btrfs/extent-io-tree.h
index 16fd403447eb..cc3037f9765e 100644
--- a/fs/btrfs/extent-io-tree.h
+++ b/fs/btrfs/extent-io-tree.h
@@ -223,6 +223,8 @@ int find_first_extent_bit(struct extent_io_tree *tree, u64 start,
struct extent_state **cached_state);
void find_first_clear_extent_bit(struct extent_io_tree *tree, u64 start,
u64 *start_ret, u64 *end_ret, unsigned bits);
+int find_contiguous_extent_bit(struct extent_io_tree *tree, u64 start,
+ u64 *start_ret, u64 *end_ret, unsigned bits);
int extent_invalidatepage(struct extent_io_tree *tree,
struct page *page, unsigned long offset);
bool btrfs_find_delalloc_range(struct extent_io_tree *tree, u64 *start,
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index d91a48d73e8f..50bbaf1c7cf0 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -1578,6 +1578,42 @@ int find_first_extent_bit(struct extent_io_tree *tree, u64 start,
return ret;
}
+/**
+ * find_contiguous_extent_bit: find a contiguous area of bits
+ * @tree - io tree to check
+ * @start - offset to start the search from
+ * @start_ret - the first offset we found with the bits set
+ * @end_ret - the final contiguous range of the bits that were set
+ *
+ * set_extent_bit anc clear_extent_bit can temporarily split contiguous ranges
+ * to set bits appropriately, and then merge them again. During this time it
+ * will drop the tree->lock, so use this helper if you want to find the actual
+ * contiguous area for given bits. We will search to the first bit we find, and
+ * then walk down the tree until we find a non-contiguous area. The area
+ * returned will be the full contiguous area with the bits set.
+ */
+int find_contiguous_extent_bit(struct extent_io_tree *tree, u64 start,
+ u64 *start_ret, u64 *end_ret, unsigned bits)
+{
+ struct extent_state *state;
+ int ret = 1;
+
+ spin_lock(&tree->lock);
+ state = find_first_extent_bit_state(tree, start, bits);
+ if (state) {
+ *start_ret = state->start;
+ *end_ret = state->end;
+ while ((state = next_state(state)) != NULL) {
+ if (state->start > (*end_ret + 1))
+ break;
+ *end_ret = state->end;
+ }
+ ret = 0;
+ }
+ spin_unlock(&tree->lock);
+ return ret;
+}
+
/**
* find_first_clear_extent_bit - find the first range that has @bits not set.
* This range could start before @start.
diff --git a/fs/btrfs/file-item.c b/fs/btrfs/file-item.c
index a73878051761..6c849e8fd5a1 100644
--- a/fs/btrfs/file-item.c
+++ b/fs/btrfs/file-item.c
@@ -51,8 +51,8 @@ void btrfs_inode_safe_disk_i_size_write(struct inode *inode, u64 new_i_size)
}
spin_lock(&BTRFS_I(inode)->lock);
- ret = find_first_extent_bit(&BTRFS_I(inode)->file_extent_tree, 0,
- &start, &end, EXTENT_DIRTY, NULL);
+ ret = find_contiguous_extent_bit(&BTRFS_I(inode)->file_extent_tree, 0,
+ &start, &end, EXTENT_DIRTY);
if (!ret && start == 0)
i_size = min(i_size, end + 1);
else
--
2.24.1
