Re: [PATCH v2 00/26] btrfs: limited subpage compressed write support

Qu Wenruo
2021-08-29T06:16:56Z

On 2021/8/29 下午1:24, Qu Wenruo wrote:
> The patchset can be fetched from github:
> https://github.com/adam900710/linux/tree/compression
>
> The branch is based on the previously submitted subpage enablement
> patchset.
> The target merge window is v5.16 or v5.17.
>
> === What's working ===
>
> Delalloc range which is fully page aligned can be compressed with
> 64K page size and 4K sector size (AKA, subpage).
>
> With current patchset, it can pass most "compress" test group, except
> btrfs/106, whose golden output is bound to 4K page size, thus test case
> needs to be updated.
>
> And as a basic requirement, 4K page size systems still pass the regular
> fstests runs.
>
> === What's not working ===
>
> Delalloc range not fully page aligned will not go through compression.
>
> That's to say, the following inode will go through different write path:
>
> 0 32K 64K 96K 128K
> |///////////////| |///////|
> | \- Will not be compressed
> |
> \- Will be compressed
>
> This will reduce the chance of compression obviously.
>
> But all involved patches will be the basis for later sector perfect
> compression support.
>
> The limitation is mostly introduced by two factors:
>
> - How we handle the locked page of a async cow delalloc range
> Currently we unlock the first page unconditionally.
> Even with the patchset, we still follows the behavior.
>
> This means we can't have two async cow range shares the same
> page.
> This can be enhanced to use subpage::writers, but the next
> problem will prevent us doing so.
>
> - No way to ensure an async cow range not to unlock the page while
> we still have delalloc range in the page
>
> This is caused by how we run delalloc range in a page.
> For regular sectorsize, it's not a problem as we have at most one
> sector for a page.
>
> But for subpage case, we can have multiple sectors in one page.
> If we submit an async cow, it may try to unlock the page while
> we are still running the next delalloc range of the page.
>
> The correct way here is to find and lock all delalloc range inside a
> page, update the subpage::writers properly, then run each delalloc
> range, so that the page won't be unlocked half way.
>
> === Patch structure ===
>
> Patch 01~04: Small and safe cleanups
> Patch 05: Make compressed readahead to be subpage compatble
> Patch 06~14: Optimize compressed read/write path to determine stripe
> boundary in a per-bio base
> Patch 15~16: Extra code refactor/cleanup for compressed path
>
> Patch 17~25: Make compressed write path to be subpage compatible
> Patch 26: Enable limited subpage compressed write support
>
> Patch 01~16 may be a good candidate for early merge, as real heavy
> lifting part starts at patch 17.
>
> While patch 01~04 are really small and safe cleanups, which can be
> merged even earlier than subpage enablement patchset.
>
> While the patches 06~14 is quite some refactor, it may be needed for the
> read-only support for compression.
> As the read-time bio split is also a critical part for read-only
> compressed data support.
>
> I don't have any good idea to push those read path fixes to v5.15
> branches for now.
> Maybe I need to craft a hot-fix for read-write support.

Forgot the changelog:

v2:
- Rebased to latest misc-next
Conflicts are caused by compact subpage bitmaps and refactored bool
parameters for @uptodate.

All tested on aarch64 machines.

Thanks,
Qu
>
> Qu Wenruo (26):
> btrfs: remove unused parameter @nr_pages in add_ra_bio_pages()
> btrfs: remove unnecessary parameter @delalloc_start for
> writepage_delalloc()
> btrfs: use async_chunk::async_cow to replace the confusing pending
> pointer
> btrfs: don't pass compressed pages to
> btrfs_writepage_endio_finish_ordered()
> btrfs: make add_ra_bio_pages() to be subpage compatible
> btrfs: introduce compressed_bio::pending_sectors to trace compressed
> bio more elegantly
> btrfs: add subpage checked bitmap to make PageChecked flag to be
> subpage compatible
> btrfs: handle errors properly inside btrfs_submit_compressed_read()
> btrfs: handle errors properly inside btrfs_submit_compressed_write()
> btrfs: introduce submit_compressed_bio() for compression
> btrfs: introduce alloc_compressed_bio() for compression
> btrfs: make btrfs_submit_compressed_read() to determine stripe
> boundary at bio allocation time
> btrfs: make btrfs_submit_compressed_write() to determine stripe
> boundary at bio allocation time
> btrfs: remove unused function btrfs_bio_fits_in_stripe()
> btrfs: refactor submit_compressed_extents()
> btrfs: cleanup for extent_write_locked_range()
> btrfs: make compress_file_range() to be subpage compatible
> btrfs: make btrfs_submit_compressed_write() to be subpage compatible
> btrfs: make end_compressed_bio_writeback() to be subpage compatble
> btrfs: make extent_write_locked_range() to be subpage compatible
> btrfs: extract uncompressed async extent submission code into a new
> helper
> btrfs: rework lzo_compress_pages() to make it subpage compatible
> btrfs: teach __extent_writepage() to handle locked page differently
> btrfs: allow page to be unlocked by btrfs_page_end_writer_lock() even
> if it's locked by plain page_lock()
> btrfs: don't run delalloc range which is beyond the locked_page to
> prevent deadlock for subpage compression
> btrfs: only allow subpage compression if the range is fully page
> aligned
>
> fs/btrfs/compression.c | 676 ++++++++++++++++++-------------
> fs/btrfs/compression.h | 4 +-
> fs/btrfs/ctree.h | 2 -
> fs/btrfs/extent_io.c | 123 ++++--
> fs/btrfs/extent_io.h | 3 +-
> fs/btrfs/file.c | 20 +-
> fs/btrfs/free-space-cache.c | 6 +-
> fs/btrfs/inode.c | 452 +++++++++++----------
> fs/btrfs/lzo.c | 280 ++++++-------
> fs/btrfs/reflink.c | 2 +-
> fs/btrfs/subpage.c | 102 ++++-
> fs/btrfs/subpage.h | 4 +
> fs/btrfs/tests/extent-io-tests.c | 12 +-
> 13 files changed, 1002 insertions(+), 684 deletions(-)
>