From: Qu Wenruo <quwenruo.btrfs@gmx.com>
To: fdmanana@kernel.org, linux-btrfs@vger.kernel.org
Subject: Re: [PATCH] Btrfs: send, skip backreference walking for extents with many references
Date: Wed, 30 Oct 2019 20:47:35 +0800 [thread overview]
Message-ID: <82eb1c76-9aa9-a666-f33c-b38763d82d23@gmx.com> (raw)
In-Reply-To: <20191030122301.25270-1-fdmanana@kernel.org>
[-- Attachment #1.1: Type: text/plain, Size: 3476 bytes --]
On 2019/10/30 下午8:23, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
>
> Backreference walking, which is used by send to figure if it can issue
> clone operations instead of write operations, can be very slow and use too
> much memory when extents have many references. This change simply skips
> backreference walking when an extent has more than 64 references, in which
> case we fallback to a write operation instead of a clone operation. This
> limit is conservative and in practice I observed no signicant slowdown
> with up to 100 references and still low memory usage up to that limit.
>
> This is a temporary workaround until there are speedups in the backref
> walking code, and as such it does not attempt to add extra interfaces or
> knobs to tweak the threshold.
>
> Reported-by: Atemu <atemu.main@gmail.com>
> Link: https://lore.kernel.org/linux-btrfs/CAE4GHgkvqVADtS4AzcQJxo0Q1jKQgKaW3JGp3SGdoinVo=C9eQ@mail.gmail.com/T/#me55dc0987f9cc2acaa54372ce0492c65782be3fa
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: Qu Wenruo <wqu@suse.com>
The workaround is much better than the old
completely-disable-reflink-detection one.
Thanks,
Qu
> ---
> fs/btrfs/send.c | 25 ++++++++++++++++++++++++-
> 1 file changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
> index 123ac54af071..518ec1265a0c 100644
> --- a/fs/btrfs/send.c
> +++ b/fs/btrfs/send.c
> @@ -25,6 +25,14 @@
> #include "compression.h"
>
> /*
> + * Maximum number of references an extent can have in order for us to attempt to
> + * issue clone operations instead of write operations. This currently exists to
> + * avoid hitting limitations of the backreference walking code (taking a lot of
> + * time and using too much memory for extents with large number of references).
> + */
> +#define SEND_MAX_EXTENT_REFS 64
> +
> +/*
> * A fs_path is a helper to dynamically build path names with unknown size.
> * It reallocates the internal buffer on demand.
> * It allows fast adding of path elements on the right side (normal path) and
> @@ -1302,6 +1310,7 @@ static int find_extent_clone(struct send_ctx *sctx,
> struct clone_root *cur_clone_root;
> struct btrfs_key found_key;
> struct btrfs_path *tmp_path;
> + struct btrfs_extent_item *ei;
> int compressed;
> u32 i;
>
> @@ -1349,7 +1358,6 @@ static int find_extent_clone(struct send_ctx *sctx,
> ret = extent_from_logical(fs_info, disk_byte, tmp_path,
> &found_key, &flags);
> up_read(&fs_info->commit_root_sem);
> - btrfs_release_path(tmp_path);
>
> if (ret < 0)
> goto out;
> @@ -1358,6 +1366,21 @@ static int find_extent_clone(struct send_ctx *sctx,
> goto out;
> }
>
> + ei = btrfs_item_ptr(tmp_path->nodes[0], tmp_path->slots[0],
> + struct btrfs_extent_item);
> + /*
> + * Backreference walking (iterate_extent_inodes() below) is currently
> + * too expensive when an extent has a large number of references, both
> + * in time spent and used memory. So for now just fallback to write
> + * operations instead of clone operations when an extent has more than
> + * a certain amount of references.
> + */
> + if (btrfs_extent_refs(tmp_path->nodes[0], ei) > SEND_MAX_EXTENT_REFS) {
> + ret = -ENOENT;
> + goto out;
> + }
> + btrfs_release_path(tmp_path);
> +
> /*
> * Setup the clone roots.
> */
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
next prev parent reply other threads:[~2019-10-30 12:47 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-30 12:23 [PATCH] Btrfs: send, skip backreference walking for extents with many references fdmanana
2019-10-30 12:47 ` Qu Wenruo [this message]
2019-11-01 14:50 ` David Sterba
2019-11-23 5:27 ` Zygo Blaxell
2019-11-23 13:33 ` Filipe Manana
2019-11-24 1:33 ` Zygo Blaxell
2019-11-25 15:31 ` Filipe Manana
2019-11-26 23:02 ` Zygo Blaxell
2019-11-27 11:42 ` Filipe Manana
2019-11-27 18:09 ` Zygo Blaxell
2019-11-27 18:20 ` Zygo Blaxell
2019-11-29 15:25 ` Filipe Manana
2019-11-26 17:52 ` Martin Raiber
2020-01-18 18:41 ` Martin Raiber
2020-01-20 10:57 ` Filipe Manana
2020-01-20 11:30 ` Martin Raiber
[not found] ` <269ad5af-c73e-ac38-5e60-39c3caa11202@urbackup.org>
2020-01-22 1:22 ` Martin Raiber
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=82eb1c76-9aa9-a666-f33c-b38763d82d23@gmx.com \
--to=quwenruo.btrfs@gmx.com \
--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).