From: Qu Wenruo <quwenruo.btrfs@gmx.com>
To: fdmanana@kernel.org, linux-btrfs@vger.kernel.org
Subject: Re: [PATCH 1/2] btrfs: drop extent maps after failed COW dio write
Date: Wed, 15 May 2024 07:45:30 +0930 [thread overview]
Message-ID: <67e9e462-b6d4-4c6d-b409-ae13ab9714a8@gmx.com> (raw)
In-Reply-To: <affef2b4ccd4d0f9a0272cd5883a5922d36bac31.1715688057.git.fdmanana@suse.com>
在 2024/5/14 23:53, fdmanana@kernel.org 写道:
> From: Filipe Manana <fdmanana@suse.com>
>
> During a COW direct IO write if the call to btrfs_extract_ordered_extent()
> fails, we don't submit any bios, cleanup the ordered extent but we leave
> the extent maps we created for the write around.
>
> These extent maps point to locations on disk that were not written to,
> since we haven't submitted a bio for them, and they are new an in the list
> of modified extents.
>
> This means that if we fsync the file after, we log file extent items based
> on those extent maps, which are invalid since they point to unwritten
> locations. If a power failure happens after the fsync, on log replay we
> get a corrupted file range.
>
> Fix this by dropping the extent maps if btrfs_extract_ordered_extent()
> failed.
>
> Fixes: b73a6fd1b1ef ("btrfs: split partial dio bios before submit")
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
Reviewed-by: Qu Wenruo <wqu@suse.com>
Just a small question inlined below.
> ---
> fs/btrfs/inode.c | 21 +++++++++++++++++++--
> 1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 5a1014122088..f04852e44123 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -7888,11 +7888,28 @@ static void btrfs_dio_submit_io(const struct iomap_iter *iter, struct bio *bio,
> * remaining pages is blocked on the outstanding ordered extent.
> */
> if (iter->flags & IOMAP_WRITE) {
> + struct btrfs_ordered_extent *oe = dio_data->ordered;
> int ret;
>
> - ret = btrfs_extract_ordered_extent(bbio, dio_data->ordered);
> + ret = btrfs_extract_ordered_extent(bbio, oe);
> if (ret) {
> - btrfs_finish_ordered_extent(dio_data->ordered, NULL,
> + /*
> + * If this is a COW write it means we created new extent
> + * maps for the range and they point to an unwritten
> + * location since we got an error and we don't submit
> + * a bio. We must drop any extent maps within the range,
> + * otherwise a fast fsync would log them and after a
> + * crash and log replay we would have file extent items
> + * that point to unwritten locations (garbage).
> + */
Is regular read also being affected?
Since the em is already inserted, and we're doing DIO, there should be
no page cache for the whole dio range (as long as we didn't fallback to
buffered IO).
In that case, the problem is not only limited to fsync, but also any read.
Thanks,
Qu
> + if (!test_bit(BTRFS_ORDERED_NOCOW, &oe->flags)) {
> + const u64 start = oe->file_offset;
> + const u64 end = start + oe->num_bytes - 1;
> +
> + btrfs_drop_extent_map_range(bbio->inode, start, end, false);
> + }
> +
> + btrfs_finish_ordered_extent(oe, NULL,
> file_offset, dip->bytes,
> !ret);
> bio->bi_status = errno_to_blk_status(ret);
next prev parent reply other threads:[~2024-05-14 22:15 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-14 14:23 [PATCH 0/2] btrfs: fix a bug in the direct IO write path for COW writes fdmanana
2024-05-14 14:23 ` [PATCH 1/2] btrfs: drop extent maps after failed COW dio write fdmanana
2024-05-14 22:15 ` Qu Wenruo [this message]
2024-05-15 9:47 ` Filipe Manana
2024-05-14 14:23 ` [PATCH 2/2] btrfs: refactor btrfs_dio_submit_io() for less nesting and indentation fdmanana
2024-05-14 22:23 ` Qu Wenruo
2024-05-15 18:51 ` [PATCH v2 0/2] btrfs: fix a bug in the direct IO write path for COW writes fdmanana
2024-05-15 18:51 ` [PATCH v2 1/2] btrfs: immediately drop extent maps after failed COW write fdmanana
2024-05-15 22:02 ` Qu Wenruo
2024-05-15 18:51 ` [PATCH v2 2/2] btrfs: make btrfs_finish_ordered_extent() return void fdmanana
2024-05-15 22:03 ` Qu Wenruo
2024-05-17 16:28 ` [PATCH v2 0/2] btrfs: fix a bug in the direct IO write path for COW writes David Sterba
2024-05-17 16:54 ` Filipe Manana
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=67e9e462-b6d4-4c6d-b409-ae13ab9714a8@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 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.