From: Qu Wenruo <wqu@suse.com>
To: linux-btrfs@vger.kernel.org
Subject: [PATCH v3 0/5] btrfs: avoid unnecessary double loop usage in RAID56
Date: Wed, 8 Jun 2022 08:34:31 +0800 [thread overview]
Message-ID: <cover.1654648358.git.wqu@suse.com> (raw)
[CHANGELOG]
v3:
- Fix a crash in btrfs/125
This is caused by the 4th patch, which the original code is only
iterating data sectors, but not touching P/Q stripes.
This triggeres a BUG_ON().
v2:
- Fix a bug in the 2nd patch that @stripe_nsector should be used
instead of @nr_sectors
This can cause btrfs/157 crash reliably
There are a lot of call patterns used in RAID56 like this:
for (stripe = 0; stripe < rbio->real_stripes; stripe++) {
for (sectornr = 0; sectornr < rbio->stripe_nsectors; sectornr++) {
struct sector_ptr *sector;
sector = sector_in_rbio(rbio, stripe, sectornr, 1);
/* Do something with the sector */
}
}
Such double for loop could cause problems for continue/break, as we have
to keep in mind which layer we're continuing or breaking out.
In fact, for most call sites, they are just iterating all the sectors in
their bytenr order.
Thus there is really no need to do such double for loop.
This patchset will convert all these unnecessary call sites to something
like this:
for (total_sector_nr = 0; total_sector_nr < rbio->nr_sectors;
total_sector_nr++) {
struct sector_ptr *sector;
sector = sector_in_rbio(rbio, stripe, sectornr, 1);
/* Do something with the sector */
}
So we don't need to bother if the break/continue is for which layer, and
reduce one indent.
There are some cases that, we may want to skip the full stripe.
In that case, it can be done by modifying @total_sector_nr manually.
Since modifying the iterator is not a good practice, every time we do
that, there will be an ASSERT() (making sure we're the first sector),
and a comment on the fact we're skipping the whole stripe.
There are still call sites doing much smaller double loop, mostly for
cases that explicitly want to handling a vertical stripe.
For those call sites, just keep them as is.
Qu Wenruo (5):
btrfs: avoid double for loop inside finish_rmw()
btrfs: avoid double for loop inside __raid56_parity_recover()
btrfs: avoid double for loop inside alloc_rbio_essential_pages()
btrfs: avoid double for loop inside raid56_rmw_stripe()
btrfs: avoid double for loop inside raid56_parity_scrub_stripe()
fs/btrfs/raid56.c | 284 ++++++++++++++++++++++++----------------------
1 file changed, 147 insertions(+), 137 deletions(-)
--
2.36.1
next reply other threads:[~2022-06-08 3:17 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-08 0:34 Qu Wenruo [this message]
2022-06-08 0:34 ` [PATCH v3 1/5] btrfs: avoid double for loop inside finish_rmw() Qu Wenruo
2022-06-08 0:34 ` [PATCH v3 2/5] btrfs: avoid double for loop inside __raid56_parity_recover() Qu Wenruo
2022-06-08 0:34 ` [PATCH v3 3/5] btrfs: avoid double for loop inside alloc_rbio_essential_pages() Qu Wenruo
2022-06-08 0:34 ` [PATCH v3 4/5] btrfs: avoid double for loop inside raid56_rmw_stripe() Qu Wenruo
2022-06-08 0:34 ` [PATCH v3 5/5] btrfs: avoid double for loop inside raid56_parity_scrub_stripe() Qu Wenruo
2022-06-14 15:40 ` [PATCH v3 0/5] btrfs: avoid unnecessary double loop usage in RAID56 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=cover.1654648358.git.wqu@suse.com \
--to=wqu@suse.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