From: David Sterba <dsterba@suse.cz>
To: fdmanana@kernel.org
Cc: linux-btrfs@vger.kernel.org
Subject: Re: [PATCH v2] Btrfs: send, fix race with transaction commits that create snapshots
Date: Wed, 12 Dec 2018 14:37:24 +0100 [thread overview]
Message-ID: <20181212133722.GV23615@twin.jikos.cz> (raw)
In-Reply-To: <20181211101945.28695-1-fdmanana@kernel.org>
On Tue, Dec 11, 2018 at 10:19:45AM +0000, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
>
> If we create a snapshot of a snapshot currently being used by a send
> operation, we can end up with send failing unexpectedly (returning
> -ENOENT error to user space for example). The following diagram shows
> how this happens.
>
> CPU 1 CPU2 CPU3
>
> btrfs_ioctl_send()
> (...)
> create_snapshot()
> -> creates snapshot of a
> root used by the send
> task
> btrfs_commit_transaction()
> create_pending_snapshot()
> __get_inode_info()
> btrfs_search_slot()
> btrfs_search_slot_get_root()
> down_read commit_root_sem
>
> get reference on eb of the
> commit root
> -> eb with bytenr == X
>
> up_read commit_root_sem
>
> btrfs_cow_block(root node)
> btrfs_free_tree_block()
> -> creates delayed ref to
> free the extent
>
> btrfs_run_delayed_refs()
> -> runs the delayed ref,
> adds extent to
> fs_info->pinned_extents
>
> btrfs_finish_extent_commit()
> unpin_extent_range()
> -> marks extent as free
> in the free space cache
>
> transaction commit finishes
>
> btrfs_start_transaction()
> (...)
> btrfs_cow_block()
> btrfs_alloc_tree_block()
> btrfs_reserve_extent()
> -> allocates extent at
> bytenr == X
> btrfs_init_new_buffer(bytenr X)
> btrfs_find_create_tree_block()
> alloc_extent_buffer(bytenr X)
> find_extent_buffer(bytenr X)
> -> returns existing eb,
> which the send task got
>
> (...)
> -> modifies content of the
> eb with bytenr == X
>
> -> uses an eb that now
> belongs to some other
> tree and no more matches
> the commit root of the
> snapshot, resuts will be
> unpredictable
>
> The consequences of this race can be various, and can lead to searches in
> the commit root performed by the send task failing unexpectedly (unable to
> find inode items, returning -ENOENT to user space, for example) or not
> failing because an inode item with the same number was added to the tree
> that reused the metadata extent, in which case send can behave incorrectly
> in the worst case or just fail later for some reason.
>
> Fix this by performing a copy of the commit root's extent buffer when doing
> a search in the context of a send operation.
>
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
Nice catch, patch added to misc-next, thanks.
prev parent reply other threads:[~2018-12-12 13:37 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-10 17:55 [PATCH] Btrfs: send, fix race with transaction commits that create snapshots fdmanana
2018-12-11 10:19 ` [PATCH v2] " fdmanana
2018-12-12 13:37 ` David Sterba [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181212133722.GV23615@twin.jikos.cz \
--to=dsterba@suse.cz \
--cc=fdmanana@kernel.org \
--cc=linux-btrfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).