All of lore.kernel.org
 help / color / mirror / Atom feed
From: Qu Wenruo <wqu@suse.com>
To: linux-btrfs@vger.kernel.org
Subject: [PATCH v7 00/13] btrfs: scrub: use a more reader friendly code to implement scrub_simple_mirror()
Date: Wed, 29 Mar 2023 07:56:07 +0800	[thread overview]
Message-ID: <cover.1680047473.git.wqu@suse.com> (raw)

This series can be found in my github repo:

https://github.com/adam900710/linux/tree/scrub_stripe

It's recommended to fetch from the repo, as our misc-next seems to
change pretty rapidly.

[Changelog]
v7:
- Fix a bug that scrub_stripe::extent_sector_bitmap is not cleared
  Exposed during my development for RAID56 new scrub code.

  Extent_sector_bitmap indicates whether the sector is utilized by
  any extent.
  If that bitmap is not cleared before running the next stripe, we
  can treat unused sectors as NODATASUM data.

  This is not a big deal for non-RAID56 profiles, as they skip empty
  stripe through other methods.
  But can be very problematic for RAID56, as they need to scrub
  data stripes then P/Q stripes.

  Such inherited bitmap makes RAID56 to scrub all those unused
  full stripes and greatly slow down the scrub.

v6:
- Fix a bug in zoned block group repair
  Exposed during my development for RAID56 new scrub code.

  There is a bug that we may use @stripe to determine if we need to
  queue a block group for repair.

  But @stripe is the last stripe we checked, it may not have any error.

  The correct way is to go through all the stripes and queue the repair
  if we found any error.

v5:
- Fix a bug that unconditionally repairs a zoned block group
  Only trigger the repair if we had any initial read failure

  Huge thanks to Johannes for the initial ZNS tests.

v4:
- Add a dedicated patch to add btrfs_bio::fs_info
  Along with dedicated allocator for scrub btrfs bios.
  The dedicated allocator is due to the fact that scrub and regular
  btrfs bios have very different mandatory members (fs_info vs inode +
  file_offset).
  For now I believe a different allocator would be better.

- Some code style change
  * No more single letter temporray structure copied from old scrub code
  * Use "for (int i = 0; ...)" when possible
  * Some new lines fixes
  * Extra brackets for tenrary operators
  * A new macro for (BTRFS_STRIPE_LEN >> PAGE_SHIFT)
  * Use enum for scrub_stripe::state bit flags
  * Use extra brackets for double shifting
  * Use explicit != 0 or == 0 comparing memcmp() results
  * Remove unnecessary ASSERT()s after btrfs_bio allocation

v3:
- Add a dedicated @fs_info member for btrfs_bio
  Unfortunately although we have a 32 bytes hole between @end_io_work and @bio,
  compiler still choose not to use that hole for whatever reasons.
  Thus this would increase the size of btrfs_bio by 4 bytes.

- Rebased to lastest misc-next

- Fix various while space error not caught by btrfs-workflow

v2:
- Use batched scrub_stripe submission
  This allows much better performance compared to the old scrub code

- Add scrub specific bio layer helpers
  This makes the scrub code to be completely rely on logical bytenr +
  mirror_num.

[PROBLEMS OF OLD SCRUB]

- Too many delayed jumps, making it hard to read
  Even starting from scrub_simple_mirror(), we have the following
  functions:

  scrub_extent()
       |
       v
  scrub_sectors()
       |
       v
  scrub_add_sector_to_rd_bio()
       | endio function
       v
  scrub_bio_end_io()
       | delayed work
       v
  scrub_bio_end_io_worker()
       |
       v
  scrub_block_complete()
       |
       v
  scrub_handle_errored_blocks()
       |
       v
  scrub_recheck_block()
       |
       v
  scrub_repair_sector_from_good_copy()

  Not to mention the hidden jumps in certain branches.

- IOPS inefficient for fragmented extents

  The real block size of scrub read is between 4K and 128K.
  If the extents are not adjacent, the blocksize drops to 4K and would
  be an IOPS disaster.

- All hardcoded to do the logical -> physical mapping by scrub itself
  No usage of any existing bio facilities.
  And even implemented a RAID56 recovery wrapper.

[NEW SCRUB_STRIPE BASED SOLUTION]

- Overall streamlined code base

  queue_scrub_stripe()
     |
     v
  scrub_find_fill_first_stripe()
     |
     v
  done

  Or

  queue_scrub_stripe()
     |
     v
  flush_scrub_stripes()
     |
     v
  scrub_submit_initial_read()
     | endio function
     v
  scrub_read_endio()
     | delayed work
     v
  scrub_stripe_read_repair_worker()
     |
     v
  scrub_verify_one_stripe()
     |
     v
  scrub_stripe_submit_repair_read()
     |
     v
  scrub_write_sectors()
     |
     v
  scrub_stripe_report_errors()

  Only one endio and delayed work, all other work are properly done in a
  sequential workflow.

- Always read in 64KiB block size
  The real blocksize of read starts at 64KiB, and ends at 512K.
  This already results a better performance even for the worst case:

  With patchset:	404.81MiB/s
  Without patchset:	369.30MiB/s

  Around 10% performance improvement on an SATA SSD.

- All logical bytenr/mirror_num based read and write

  With the new single stripe fast path in btrfs_submit_bio(), scrub can
  reuse most of the bio layer code, result much simpler scrub code.

[TODO]

- More testing on zoned devices
  Now the patchset can already pass all scrub/replace groups with
  regular devices.

- Cleanup on RAID56 path
  Already done in the github repo.
  Still needs to do some patch split as the cleanup itself is
  twice the size of the submitted series already.

Qu Wenruo (13):
  btrfs: scrub: use dedicated super block verification function to scrub
    one super block
  btrfs: introduce a new allocator for scrub specific btrfs_bio
  btrfs: introduce a new helper to submit read bio for scrub
  btrfs: introduce a new helper to submit write bio for scrub
  btrfs: scrub: introduce the structure for new BTRFS_STRIPE_LEN based
    interface
  btrfs: scrub: introduce a helper to find and fill the sector info for
    a scrub_stripe
  btrfs: scrub: introduce a helper to verify one metadata
  btrfs: scrub: introduce a helper to verify one scrub_stripe
  btrfs: scrub: introduce the main read repair worker for scrub_stripe
  btrfs: scrub: introduce a writeback helper for scrub_stripe
  btrfs: scrub: introduce error reporting functionality for scrub_stripe
  btrfs: scrub: introduce the helper to queue a stripe for scrub
  btrfs: scrub: switch scrub_simple_mirror() to scrub_stripe
    infrastructure

 fs/btrfs/bio.c       |  165 ++++-
 fs/btrfs/bio.h       |   22 +-
 fs/btrfs/file-item.c |    9 +-
 fs/btrfs/file-item.h |    3 +-
 fs/btrfs/raid56.c    |    2 +-
 fs/btrfs/scrub.c     | 1662 ++++++++++++++++++++++++++++++------------
 6 files changed, 1378 insertions(+), 485 deletions(-)

-- 
2.39.2


             reply	other threads:[~2023-03-28 23:57 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-28 23:56 Qu Wenruo [this message]
2023-03-28 23:56 ` [PATCH v7 01/13] btrfs: scrub: use dedicated super block verification function to scrub one super block Qu Wenruo
2023-03-29  0:29   ` Anand Jain
2023-03-29  9:20   ` Johannes Thumshirn
2023-03-29  9:53     ` Qu Wenruo
2023-03-29 23:25   ` Christoph Hellwig
2023-03-28 23:56 ` [PATCH v7 02/13] btrfs: introduce a new allocator for scrub specific btrfs_bio Qu Wenruo
2023-03-29 23:32   ` Christoph Hellwig
2023-03-29 23:39     ` Qu Wenruo
2023-03-29 23:47       ` Christoph Hellwig
2023-03-29 23:51         ` Qu Wenruo
2023-03-29 23:54           ` Christoph Hellwig
2023-03-28 23:56 ` [PATCH v7 03/13] btrfs: introduce a new helper to submit read bio for scrub Qu Wenruo
2023-03-29 23:33   ` Christoph Hellwig
2023-03-29 23:41     ` Qu Wenruo
2023-03-30  6:43     ` Qu Wenruo
2023-03-30 21:50       ` Christoph Hellwig
2023-03-28 23:56 ` [PATCH v7 04/13] btrfs: introduce a new helper to submit write " Qu Wenruo
2023-03-29 23:33   ` Christoph Hellwig
2023-03-30  6:47     ` Qu Wenruo
2023-03-30 22:13       ` Christoph Hellwig
2023-03-31  0:48         ` Qu Wenruo
2023-03-31  0:56         ` Qu Wenruo
2023-03-28 23:56 ` [PATCH v7 05/13] btrfs: scrub: introduce the structure for new BTRFS_STRIPE_LEN based interface Qu Wenruo
2023-03-28 23:56 ` [PATCH v7 06/13] btrfs: scrub: introduce a helper to find and fill the sector info for a scrub_stripe Qu Wenruo
2023-03-28 23:56 ` [PATCH v7 07/13] btrfs: scrub: introduce a helper to verify one metadata Qu Wenruo
2023-03-28 23:56 ` [PATCH v7 08/13] btrfs: scrub: introduce a helper to verify one scrub_stripe Qu Wenruo
2023-03-28 23:56 ` [PATCH v7 09/13] btrfs: scrub: introduce the main read repair worker for scrub_stripe Qu Wenruo
2023-03-28 23:56 ` [PATCH v7 10/13] btrfs: scrub: introduce a writeback helper " Qu Wenruo
2023-03-28 23:56 ` [PATCH v7 11/13] btrfs: scrub: introduce error reporting functionality " Qu Wenruo
2023-03-28 23:56 ` [PATCH v7 12/13] btrfs: scrub: introduce the helper to queue a stripe for scrub Qu Wenruo
2023-03-28 23:56 ` [PATCH v7 13/13] btrfs: scrub: switch scrub_simple_mirror() to scrub_stripe infrastructure Qu Wenruo

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.1680047473.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 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.