Messages in this thread | | | From | "Chris Mason" <> | Subject | Re: [PATCH btrfs/for-next] btrfs: fix fatal extent_buffer readahead vs releasepage race | Date | Wed, 17 Jun 2020 13:43:33 -0400 |
| |
On 17 Jun 2020, at 13:20, Filipe Manana wrote:
> On Wed, Jun 17, 2020 at 5:32 PM Boris Burkov <boris@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
| |