All of lore.kernel.org
 help / color / mirror / Atom feed
From: Josef Bacik <josef@toxicpanda.com>
To: fdmanana@kernel.org
Cc: linux-btrfs@vger.kernel.org
Subject: Re: [PATCH] btrfs: send: fix a failure when looking for data backrefs after relocation
Date: Thu, 2 Dec 2021 14:28:49 -0500	[thread overview]
Message-ID: <YakecWBMcRKlPdGa@localhost.localdomain> (raw)
In-Reply-To: <829076d580be74f270e740f8dded6fda45390311.1638440202.git.fdmanana@suse.com>

On Thu, Dec 02, 2021 at 10:21:43AM +0000, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
> 
> During a send, when trying to find roots from which to clone data extents,
> if the leaf of our file extent item was obtained before relocation for a
> data block group finished, we can end up trying to lookup for backrefs
> for an extent location (file extent item's disk_bytenr) that is not in
> use anymore. That is, the extent was reallocated and the transaction used
> for the relocation was committed. This makes the backref lookup not find
> anything and we fail at find_extent_clone() with -EIO and log an error
> message like the following:
> 
>   [ 7642.897365] BTRFS error (device sdc): did not find backref in send_root. inode=881, offset=2592768, disk_byte=1292025856 found extent=1292025856
> 
> This is because we are checking if relocation happened after we check if
> we found the backref for the file extent item we are processing. We should
> do it before, and in case relocation happened, do not attempt to clone and
> instead fallback to issuing write commands, which will read the correct
> data from the new extent location. The current check is being done too
> late, so fix this by moving it to right after we do the backref lookup and
> before checking if we found our own backref.
> 
> Signed-off-by: Filipe Manana <fdmanana@suse.com>

I'm not against this in principal, but won't we come all the way back out of
this loop and re-search higher up because things changed?  Can we just do a
-EAGAIN, come out and re-search down to this key so we can still do the clone
properly?  If we can't then this is reasonable, but I'd like to avoid blowing up
a send stream because relocation was running if at all possible.

Josef

  reply	other threads:[~2021-12-02 19:29 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-02 10:21 [PATCH] btrfs: send: fix a failure when looking for data backrefs after relocation fdmanana
2021-12-02 19:28 ` Josef Bacik [this message]
2021-12-03 11:13   ` Filipe Manana
2021-12-06 17:52 ` David Sterba

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=YakecWBMcRKlPdGa@localhost.localdomain \
    --to=josef@toxicpanda.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.