On 17 Jun 2020, at 13:20, Filipe Manana wrote:
On Wed, Jun 17, 2020 at 5:32 PM Boris Burkov <bo...@bur.io> wrote:
---
fs/btrfs/extent_io.c | 45
++++++++++++++++++++++++++++----------------
1 file changed, 29 insertions(+), 16 deletions(-)
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index c59e07360083..f6758ebbb6a2 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -3927,6 +3927,11 @@ static noinline_for_stack int
write_one_eb(struct extent_buffer *eb,
clear_bit(EXTENT_BUFFER_WRITE_ERR, &eb->bflags);
num_pages = num_extent_pages(eb);
atomic_set(&eb->io_pages, num_pages);
+ /*
+ * It is possible for releasepage to clear the TREE_REF bit
before we
+ * set io_pages. See check_buffer_tree_ref for a more
detailed comment.
+ */
+ check_buffer_tree_ref(eb);
This is a whole different case from the one described in the
changelog, as this is in the write path.
Why do we need this one?
This was Josef’s idea, but I really like the symmetry. You set
io_pages, you do the tree_ref dance. Everyone fiddling with the write
back bit right now correctly clears writeback after doing the atomic_dec
on io_pages, but the race is tiny and prone to getting exposed again by
shifting code around. Tree ref checks around io_pages are the most
reliable way to prevent this bug from coming back again later.
-chris
