From: Christoph Hellwig <hch@lst.de>
To: David Sterba <dsterba@suse.cz>
Cc: Johannes Thumshirn <Johannes.Thumshirn@wdc.com>,
Christoph Hellwig <hch@lst.de>, Chris Mason <clm@fb.com>,
Josef Bacik <josef@toxicpanda.com>,
David Sterba <dsterba@suse.com>,
"linux-btrfs@vger.kernel.org" <linux-btrfs@vger.kernel.org>
Subject: Re: [PATCH 04/20] btrfs: always read the entire extent_buffer
Date: Mon, 20 Mar 2023 06:46:00 +0100 [thread overview]
Message-ID: <20230320054600.GA18708@lst.de> (raw)
In-Reply-To: <20230317231609.GE10580@twin.jikos.cz>
On Sat, Mar 18, 2023 at 12:16:09AM +0100, David Sterba wrote:
> I remember one bug that was solved by splitting the first loop into two,
> one locking all pages first and then checking the uptodate status, the
> comment is explaining that.
>
> 2571e739677f ("Btrfs: fix memory leak in reading btree blocks")
>
> As you can see in the changelog it's a bit convoluted race, the number
> of loops should not matter if they're there for correctness reasons. I
> haven't checked the final code but I doubt it's equivalent and may
> introduce subtle bugs.
The patch we're replying to would have actually fixed that above
bug on it's own, and thus have also fixed the above issue. The
problem it solves was that num_pages could be smaller than the
actual number of pages in the extent_buffer. With this change,
we always read all pages and thus can't have a wrong ->io_pages.
In fact a later patch removes ->io_pages entirely. Even better
at the end of the series the locking between different reads moves
to the extent_buffer level, so the above race is fundamentally
fixed at a higher level and there won't be an extra read at all.
next prev parent reply other threads:[~2023-03-20 5:46 UTC|newest]
Thread overview: 74+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-09 9:05 simplify extent_buffer reading and writing Christoph Hellwig
2023-03-09 9:05 ` [PATCH 01/20] btrfs: mark extent_buffer_under_io static Christoph Hellwig
2023-03-09 11:06 ` Johannes Thumshirn
2023-03-10 7:26 ` Qu Wenruo
2023-03-09 9:05 ` [PATCH 02/20] btrfs: move setting the buffer uptodate out of validate_extent_buffer Christoph Hellwig
2023-03-09 11:10 ` Johannes Thumshirn
2023-03-10 7:27 ` Qu Wenruo
2023-03-09 9:05 ` [PATCH 03/20] btrfs: merge verify_parent_transid and btrfs_buffer_uptodate Christoph Hellwig
2023-03-09 11:17 ` Johannes Thumshirn
2023-03-09 15:21 ` Christoph Hellwig
2023-03-10 7:28 ` Qu Wenruo
2023-03-09 9:05 ` [PATCH 04/20] btrfs: always read the entire extent_buffer Christoph Hellwig
2023-03-09 11:29 ` Johannes Thumshirn
2023-03-09 15:21 ` Christoph Hellwig
2023-03-14 6:09 ` Christoph Hellwig
2023-03-17 23:16 ` David Sterba
2023-03-20 5:46 ` Christoph Hellwig [this message]
2023-03-10 7:35 ` Qu Wenruo
2023-03-09 9:05 ` [PATCH 05/20] btrfs: simplify extent buffer reading Christoph Hellwig
2023-03-09 11:59 ` Johannes Thumshirn
2023-03-10 7:42 ` Qu Wenruo
2023-03-10 7:47 ` Christoph Hellwig
2023-03-10 8:02 ` Qu Wenruo
2023-03-10 8:03 ` Christoph Hellwig
2023-03-10 8:07 ` Qu Wenruo
2023-03-10 8:15 ` Christoph Hellwig
2023-03-10 9:14 ` Qu Wenruo
2023-03-10 10:54 ` Filipe Manana
2023-03-10 11:12 ` Qu Wenruo
2023-03-09 9:05 ` [PATCH 06/20] btrfs: remove the mirror_num argument to btrfs_submit_compressed_read Christoph Hellwig
2023-03-09 12:58 ` Johannes Thumshirn
2023-03-09 9:05 ` [PATCH 07/20] btrfs: simplify the read_extent_buffer end_io handler Christoph Hellwig
2023-03-09 13:08 ` Johannes Thumshirn
2023-03-10 8:14 ` Qu Wenruo
2023-03-10 8:17 ` Christoph Hellwig
2023-03-10 8:30 ` Qu Wenruo
2023-03-10 9:30 ` Qu Wenruo
2023-03-09 9:05 ` [PATCH 08/20] btrfs: do not try to unlock the extent for non-subpage metadata reads Christoph Hellwig
2023-03-09 13:13 ` Johannes Thumshirn
2023-03-09 9:05 ` [PATCH 09/20] btrfs: return bool from lock_extent_buffer_for_io Christoph Hellwig
2023-03-09 13:17 ` Johannes Thumshirn
2023-03-09 9:05 ` [PATCH 10/20] btrfs: submit a writeback bio per extent_buffer Christoph Hellwig
2023-03-09 13:35 ` Johannes Thumshirn
2023-03-09 9:05 ` [PATCH 11/20] btrfs: move page locking from lock_extent_buffer_for_io to write_one_eb Christoph Hellwig
2023-03-09 13:46 ` Johannes Thumshirn
2023-03-09 9:05 ` [PATCH 12/20] btrfs: simplify extent buffer writing Christoph Hellwig
2023-03-09 14:00 ` Johannes Thumshirn
2023-03-09 15:22 ` Christoph Hellwig
2023-03-10 8:34 ` Qu Wenruo
2023-03-10 8:41 ` Christoph Hellwig
2023-03-09 9:05 ` [PATCH 13/20] btrfs: simplify the extent_buffer write end_io handler Christoph Hellwig
2023-03-09 14:10 ` Johannes Thumshirn
2023-03-09 15:22 ` Christoph Hellwig
2023-03-10 8:44 ` Qu Wenruo
2023-03-10 11:47 ` Christoph Hellwig
2023-03-09 9:05 ` [PATCH 14/20] btrfs: simplify btree block checksumming Christoph Hellwig
2023-03-09 15:51 ` Johannes Thumshirn
2023-03-10 8:57 ` Qu Wenruo
2023-03-09 9:05 ` [PATCH 15/20] btrfs: remove the io_pages field in struct extent_buffer Christoph Hellwig
2023-03-09 16:01 ` Johannes Thumshirn
2023-03-10 8:53 ` Qu Wenruo
2023-03-10 11:50 ` Christoph Hellwig
2023-03-09 9:05 ` [PATCH 16/20] btrfs: stop using PageError for extent_buffers Christoph Hellwig
2023-03-09 16:05 ` Johannes Thumshirn
2023-03-09 9:05 ` [PATCH 17/20] btrfs: don't check for uptodate pages in read_extent_buffer_pages Christoph Hellwig
2023-03-09 16:10 ` Johannes Thumshirn
2023-03-10 9:08 ` Qu Wenruo
2023-03-10 11:54 ` Christoph Hellwig
2023-03-14 6:12 ` Christoph Hellwig
2023-03-09 9:05 ` [PATCH 18/20] btrfs: stop using lock_extent in btrfs_buffer_uptodate Christoph Hellwig
2023-03-09 9:05 ` [PATCH 19/20] btrfs: use per-buffer locking for extent_buffer reading Christoph Hellwig
2023-03-09 17:12 ` Johannes Thumshirn
2023-03-09 9:05 ` [PATCH 20/20] btrfs: merge write_one_subpage_eb into write_one_eb Christoph Hellwig
2023-03-09 17:28 ` Johannes Thumshirn
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=20230320054600.GA18708@lst.de \
--to=hch@lst.de \
--cc=Johannes.Thumshirn@wdc.com \
--cc=clm@fb.com \
--cc=dsterba@suse.com \
--cc=dsterba@suse.cz \
--cc=josef@toxicpanda.com \
--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