On Wed, Jun 23, 2021 at 01:55:21PM +0800, Qu Wenruo wrote:

This patchset is based on my previously submitted compression refactor,
which is further based on subpage patchset.

It can be fetched from the following repo:
https://github.com/adam900710/linux/tree/compression

It only has been lightly tested, no stress test/full fstests run yet.
The reason for such a unstable patchset is to get feedback on how to
continue the development, aka the feedback for the last patch.

The first 7 patches are mostly cleanups to make compression path to be
more subpage compatible.

The RFC is for the last patch, which will enable subpage compression in
a pretty controversial way.

The reason is copied from that patch:

This mechanism allows btrfs to continue handling next ranges, without
waiting for the time consuming compression.

But this has a problem for subpage case, as we could have the following
delalloc range for a page:

0		32K		64K
|	|///////|	|///////|
		\- A		\- B

In above case, if we pass both range to cow_file_range_async(), both
range A and range B will try to unlock the full page [0, 64K).

And which finishes later than the other range will try to do other page
operatioins like end_page_writeback() on a unlocked page, triggering VM
layer BUG_ON().

Currently I don't have any perfect solution to this, but two
workarounds:

- Only allow compression for fully page aligned range

  This is what I did in this patch.
  By this, the compressed range can exclusively lock the first page
  (and other pages), so they are completely safe to do whatever they
  want.
  The problem is, we will not compress a lot of small writes.
  This is especially problematic as our target page size is 64K, not
  a small size.

- Make cow_file_range_async() to behave like cow_file_range() for
  subpage

  This needs several behavier change, and are all subpage specific:
  * Skip the first page of the range when finished
    Just like cow_file_range()
  * Have a way to wait for the async_cow to finish before handling the
    next delalloc range

  The biggest problem here is the performance impact.
  Although by this we can compress all sector aligned ranges, we will
  waste time waiting for the async_cow to finish.
  This is completely denying the meaning of "async" part.
  Now to mention there are tons of code needs to be changed.

Thus I choose the current way to only compress ranges which is fully
page aligned.
The cost is we will skip a lot of small writes for 64K page size.

Any feedback on this part would be pretty helpful.

As another step, progressing towards full subpage support I think that
the limiting compression to the full 64k page is acceptable. Better than
nothing and without intrusive changes described above.

There still might be some odd case even with the whole page, I'm not
sure how exactly it would work with 4k/64k but there are some checks
inside compression anyway if the size is getting smaller and then
bailing out uncompressed anyway. So in general fallback to uncompressed
data happens already.