lkml.org 
[lkml]   [2020]   [Jun]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH btrfs/for-next] btrfs: fix fatal extent_buffer readahead vs releasepage race
Date
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

\
 
 \ /
  Last update: 2020-06-17 19:44    [W:0.110 / U:0.768 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site