[PATCH 0/3] btrfs: fixes for incoming sector perfect compression support

[PATCH 0/3] btrfs: fixes for incoming sector perfect compression support

[Qu Wenruo]

Currently if a btrfs is mount with sector size < page size (aka,
subpage), then compression is only limited to range that is fully page
aligned.

This is mostly caused by the asynchonously submission for compression.

With recent changes in the page writeback path, we're closer and closer
to the sector perfect compression support for subpage.

Locally I have already enabled such support to shake out the remaining
bugs.

So far 2 obvious bugs are exposed by my local fstests runs.
They all share the same cause, incorrect assumption that one page is one
sector, thus some pointer is always increased by PAGE_SIZE, which is not
correct.

Fix both of them with a single line fix, then add an ASSERT() to ensure
the total read-in bytes never exceed the input range.

There is a remaining hang reproduced by btrfs/062, which I'm still
debugging, but I think sector perfect compression support is not that far
away anymore.

Qu Wenruo (3):
  btrfs: zlib: make the compression path to handle sector size < page
    size
  btrfs: zstd: make the compression path to handle sector size < page
    size
  btrfs: compression: add an ASSERT() to ensure the read-in length is
    sane

 fs/btrfs/compression.c | 3 +++
 fs/btrfs/zlib.c        | 2 +-
 fs/btrfs/zstd.c        | 2 +-
 3 files changed, 5 insertions(+), 2 deletions(-)

-- 
2.46.0

[PATCH 1/3] btrfs: zlib: make the compression path to handle sector size < page size

[Qu Wenruo]

Inside zlib_compress_folios(), each time we switch the input page cache,
the @start is increased by PAGE_SIZE.

But for the incoming compression support for sector size < page size
(previously we support compression only when the range is fully page
aligned), this is not going to handle the following case:

    0          32K         64K          96K
    |          |///////////||///////////|

@start has the initial value 32K, indicating the start filepos of the
to-be-compressed range.

And when grabbing the first page as input, we always call "start +=
PAGE_SIZE;".

But since @start is starting at 32K, it will be increased by 64K,
resulting it to be 96K for the next range, causing incorrect input range
and corruption for the future subpage compression.

Fix it by only increase @start by the input size.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/zlib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/zlib.c b/fs/btrfs/zlib.c
index 100abc00b794..ddf0d5a448a7 100644
--- a/fs/btrfs/zlib.c
+++ b/fs/btrfs/zlib.c
@@ -194,7 +194,7 @@ int zlib_compress_folios(struct list_head *ws, struct address_space *mapping,
 				pg_off = offset_in_page(start);
 				cur_len = btrfs_calc_input_length(orig_end, start);
 				data_in = kmap_local_folio(in_folio, pg_off);
-				start += PAGE_SIZE;
+				start += cur_len;
 				workspace->strm.next_in = data_in;
 				workspace->strm.avail_in = cur_len;
 			}
-- 
2.46.0

[PATCH 2/3] btrfs: zstd: make the compression path to handle sector size < page size

[Qu Wenruo]

Inside zstd_compress_folios(), after exhausted one input page, we need
to switch to the next page as input.

However when counting the total input bytes (@tot_in), we always increase
it by PAGE_SIZE.

For the following case, it can cause incorrect value:

        0          32K         64K          96K
        |          |///////////||///////////|

After compressing range [32K, 64K), we switch to the next page, and
increasing @tot_in by 64K, while we only read 32K.

This will cause the @total_in to return a value larger than the input
length.

Fix it by only increase @tot_in by the input size.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/zstd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/zstd.c b/fs/btrfs/zstd.c
index 866607fd3e58..15f8a83165a3 100644
--- a/fs/btrfs/zstd.c
+++ b/fs/btrfs/zstd.c
@@ -495,7 +495,7 @@ int zstd_compress_folios(struct list_head *ws, struct address_space *mapping,
 
 		/* Check if we need more input */
 		if (workspace->in_buf.pos == workspace->in_buf.size) {
-			tot_in += PAGE_SIZE;
+			tot_in += workspace->in_buf.size;
 			kunmap_local(workspace->in_buf.src);
 			workspace->in_buf.src = NULL;
 			folio_put(in_folio);
-- 
2.46.0

[PATCH 3/3] btrfs: compression: add an ASSERT() to ensure the read-in length is sane

[Qu Wenruo]

There are already two bugs (one in zlib, one in zstd) that involved
compression path is not handling sector size < page size cases well.

So it makes more sense to make sure that btrfs_compress_folios() returns

Since we already have two bugs (one in zlib, one in zstd) in the
compression path resulting the @total_in be to larger than the
to-be-compressed range length, there is enough reason to add an ASSERT()
to make sure the total read-in length doesn't exceed the input length.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/compression.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 90aef2627ca2..6e9c4a5e0d51 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -1030,6 +1030,7 @@ int btrfs_compress_folios(unsigned int type_level, struct address_space *mapping
 {
 	int type = btrfs_compress_type(type_level);
 	int level = btrfs_compress_level(type_level);
+	const unsigned long orig_len = *total_out;
 	struct list_head *workspace;
 	int ret;
 
@@ -1037,6 +1038,8 @@ int btrfs_compress_folios(unsigned int type_level, struct address_space *mapping
 	workspace = get_workspace(type, level);
 	ret = compression_compress_pages(type, workspace, mapping, start, folios,
 					 out_folios, total_in, total_out);
+	/* The total read-in bytes should be no larger than the input. */
+	ASSERT(*total_in <= orig_len);
 	put_workspace(type, workspace);
 	return ret;
 }
-- 
2.46.0