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

[Qu Wenruo <wqu@suse.com>]

On 2021/7/16 下午5:11, Qu Wenruo wrote:
> 
> 
> On 2021/7/13 下午2:14, 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.
> 
> It turns out that, btrfs/160 has a very high chance to crash due to 
> ordered extent tree inconsistency.
> This is only exposed when running with "-o compress" mount option.

To make things more weird, if I disable space cache then run the test 
again, it no longer crashes anymore.

Recent debug also shows the problem is inside the ordered extent 
cleanup, for *space cache*.

By somehow, it seems with subpage compression, we're creating larger v1 
free space cache file (this behavior itself can be a bug though), thus 
it's more vulnerable to IO error half way with large ordered extent 
submitted.

So this is a bug in generic data writeback path, but some how only 
subpage compression makes it easier to trigger.

Thanks,
Qu

> 
> Will fix all the bugs exposed during full fstests run with "-o compress" 
> mount option.
> 
> Thanks,
> Qu
> 
>>
>> 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~26:    Make compressed write path to be subpage compatible
>> Patch 27:    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.
>>
>>
>> Qu Wenruo (27):
>>    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: allow subpage to compress a range which only covers one page
>>    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           | 678 ++++++++++++++++++-------------
>>   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                 | 455 +++++++++++----------
>>   fs/btrfs/lzo.c                   | 280 ++++++-------
>>   fs/btrfs/reflink.c               |   2 +-
>>   fs/btrfs/subpage.c               |  85 ++++
>>   fs/btrfs/subpage.h               |  10 +
>>   fs/btrfs/tests/extent-io-tests.c |  12 +-
>>   13 files changed, 996 insertions(+), 684 deletions(-)
>>
>