On 2021/3/25 下午8:20, Neal Gompa wrote:

On Thu, Mar 25, 2021 at 3:17 AM Qu Wenruo [off-list ref] wrote:

quoted

This patchset can be fetched from the following github repo, along with
the full subpage RW support:
https://github.com/adam900710/linux/tree/subpage

This patchset is for metadata read write support.

[FULL RW TEST]
Since the data write path is not included in this patchset, we can't
really test the patchset itself, but anyone can grab the patch from
github repo and do fstests/generic tests.

But at least the full RW patchset can pass -g generic/quick -x defrag
for now.

There are some known issues:

- Defrag behavior change
   Since current defrag is doing per-page defrag, to support subpage
   defrag, we need some change in the loop.
   E.g. if a page has both hole and regular extents in it, then defrag
   will rewrite the full 64K page.

   Thus for now, defrag related failure is expected.
   But this should only cause behavior difference, no crash nor hang is
   expected.

- No compression support yet
   There are at least 2 known bugs if forcing compression for subpage
   * Some hard coded PAGE_SIZE screwing up space rsv
   * Subpage ASSERT() triggered
     This is because some compression code is unlocking locked_page by
     calling extent_clear_unlock_delalloc() with locked_page == NULL.
   So for now compression is also disabled.

- Inode nbytes mismatch
   Still debugging.
   The fastest way to trigger is fsx using the following parameters:

     fsx -l 262144 -o 65536 -S 30073 -N 256 -R -W $mnt/file > /tmp/fsx

   Which would cause inode nbytes differs from expected value and
   triggers btrfs check error.

[DIFFERENCE AGAINST REGULAR SECTORSIZE]
The metadata part in fact has more new code than data part, as it has
some different behaviors compared to the regular sector size handling:

- No more page locking
   Now metadata read/write relies on extent io tree locking, other than
   page locking.
   This is to allow behaviors like read lock one eb while also try to
   read lock another eb in the same page.
   We can't rely on page lock as now we have multiple extent buffers in
   the same page.

- Page status update
   Now we use subpage wrappers to handle page status update.

- How to submit dirty extent buffers
   Instead of just grabbing extent buffer from page::private, we need to
   iterate all dirty extent buffers in the page and submit them.

[CHANGELOG]
v2:
- Rebased to latest misc-next
   No conflicts at all.

- Add new sysfs interface to grab supported RO/RW sectorsize
   This will allow mkfs.btrfs to detect unmountable fs better.

- Use newer naming schema for each patch
   No more "extent_io:" or "inode:" schema anymore.

- Move two pure cleanups to the series
   Patch 2~3, originally in RW part.

- Fix one uninitialized variable
   Patch 6.

v3:
- Rename the sysfs to supported_sectorsizes

- Rebased to latest misc-next branch
   This removes 2 cleanup patches.

- Add new overview comment for subpage metadata

Qu Wenruo (13):
   btrfs: add sysfs interface for supported sectorsize
   btrfs: use min() to replace open-code in btrfs_invalidatepage()
   btrfs: remove unnecessary variable shadowing in btrfs_invalidatepage()
   btrfs: refactor how we iterate ordered extent in
     btrfs_invalidatepage()
   btrfs: introduce helpers for subpage dirty status
   btrfs: introduce helpers for subpage writeback status
   btrfs: allow btree_set_page_dirty() to do more sanity check on subpage
     metadata
   btrfs: support subpage metadata csum calculation at write time
   btrfs: make alloc_extent_buffer() check subpage dirty bitmap
   btrfs: make the page uptodate assert to be subpage compatible
   btrfs: make set/clear_extent_buffer_dirty() to be subpage compatible
   btrfs: make set_btree_ioerr() accept extent buffer and to be subpage
     compatible
   btrfs: add subpage overview comments

  fs/btrfs/disk-io.c   | 143 ++++++++++++++++++++++++++++++++++---------
  fs/btrfs/extent_io.c | 127 ++++++++++++++++++++++++++++----------
  fs/btrfs/inode.c     | 128 ++++++++++++++++++++++----------------
  fs/btrfs/subpage.c   | 127 ++++++++++++++++++++++++++++++++++++++
  fs/btrfs/subpage.h   |  17 +++++
  fs/btrfs/sysfs.c     |  15 +++++
  6 files changed, 441 insertions(+), 116 deletions(-)

--
2.30.1

Why wouldn't we just integrate full read-write support with the
caveats as described now? It seems to be relatively reasonable to do
that, and this patch set is essentially unusable without the rest of
it that does enable full read-write support.

The metadata part is much more stable than data path (almost not touched
for several months), and the metadata part already has some difference
in its behavior, which needs review.

You point makes some sense, but I still don't believe pushing a super
large patchset does any help for the review.

If you want to test, you can grab the branch from the github repo.
If you want to review, the mails are all here for review.

In fact, we used to have subpage support sent as a big patchset from IBM
guys, but the result is only some preparation patches get merged, and
nothing more.

Using this multi-series method, we're already doing better work and
received more testing (to ensure regular sectorsize is not affected at
least).

Thanks,
Qu