[PATCH 0/2] btrfs: fix unpaired compr folio allocation/freeing

[PATCH 0/2] btrfs: fix unpaired compr folio allocation/freeing

[Qu Wenruo]

With the recent *_compressed_bio() rework, we're using compressed_bio
everywhere for reads/writes (including regular and encoded).

However this requires us to allocate/free folios using
btrfs_alloc_compr_folio() and btrfs_free_compr_folio().

Not using the later to release a folio allocated by
btrfs_alloc_compr_folio() can screw up the LRU list.

Fix the unpaired calls as a hot fix.

For the long run, I'd like to remove btrfs_(alloc|free)_compr_folios()
with regular folio_(alloc|put)() instead.
As I do not think the compr pool is that helpful compared to the extra
maintanance needed.

Qu Wenruo (2):
  btrfs: fix the incorrect freeing of a compression folio in
    btrfs_submit_compressed_read()
  btrfs: fix the incorrect freeing of a compression folio in
    btrfs_do_encoded_write()

 fs/btrfs/compression.c | 2 +-
 fs/btrfs/inode.c       | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

-- 
2.53.0

[PATCH 1/2] btrfs: fix the incorrect freeing of a compression folio in btrfs_submit_compressed_read()

[Qu Wenruo]

[BUG]
Commit dafcfa1c8e37 ("btrfs: get rid of compressed_folios[] usage for
compressed read") changed how we allocate folios for a compressed read.

Previously we allocate an array of folios for the compressed read.
But now we require all folios to be allocated and freed by
btrfs_alloc_compr_folios() and btrfs_free_compr_folios() to use the
cached folio pool.

Unfortunately we only use btrfs_alloc_compr_folio() but not pairing it
with btrfs_free_compr_folio().

This can lead to problems as a compr folio has maintained its own RCU
list, not releasing it from the RCU list can lead to various problems.

[FIX]
Use btrfs_free_compr_folio() to replace the regular folio_put() call.

Fixes: dafcfa1c8e37 ("btrfs: get rid of compressed_folios[] usage for compressed read")
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/compression.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 192f133d9eb5..dd004b1cc4ef 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -576,7 +576,7 @@ void btrfs_submit_compressed_read(struct btrfs_bio *bbio)
 
 		ret = bio_add_folio(&cb->bbio.bio, folio, cur_len, 0);
 		if (unlikely(!ret)) {
-			folio_put(folio);
+			btrfs_free_compr_folio(folio);
 			ret = -EINVAL;
 			goto out_free_bio;
 		}
-- 
2.53.0

[PATCH 2/2] btrfs: fix the incorrect freeing of a compression folio in btrfs_do_encoded_write()

[Qu Wenruo]

[BUG]
Commit e1bc83f8b157 ("btrfs: get rid of compressed_folios[] usage for
encoded writes") changed how we allocate folios for encoded writes.

Previously we allocate an array of folios for the encoded write.
But now we require all folios to be allocated and freed by
btrfs_alloc_compr_folios() and btrfs_free_compr_folios() to use the
cached folio pool.

Unfortunately we only use btrfs_alloc_compr_folio() but not pairing it
with btrfs_free_compr_folio().

This can lead to problems as a compr folio has maintained its own RCU
list, not releasing it from the RCU list can lead to various problems.

[FIX]
Use btrfs_free_compr_folio() to replace the regular folio_put() call.

Fixes: e1bc83f8b157 ("btrfs: get rid of compressed_folios[] usage for encoded writes")
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/inode.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index ea3746c14760..8702247e4db0 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -9989,7 +9989,7 @@ ssize_t btrfs_do_encoded_write(struct kiocb *iocb, struct iov_iter *from,
 		ret = copy_from_iter(kaddr, bytes, from);
 		kunmap_local(kaddr);
 		if (ret != bytes) {
-			folio_put(folio);
+			btrfs_free_compr_folio(folio);
 			ret = -EFAULT;
 			goto out_cb;
 		}
@@ -9997,7 +9997,7 @@ ssize_t btrfs_do_encoded_write(struct kiocb *iocb, struct iov_iter *from,
 			folio_zero_range(folio, bytes, round_up(bytes, blocksize) - bytes);
 		ret = bio_add_folio(&cb->bbio.bio, folio, round_up(bytes, blocksize), 0);
 		if (unlikely(!ret)) {
-			folio_put(folio);
+			btrfs_free_compr_folio(folio);
 			ret = -EINVAL;
 			goto out_cb;
 		}
-- 
2.53.0

Re: [PATCH 0/2] btrfs: fix unpaired compr folio allocation/freeing

[Qu Wenruo]

在 2026/2/28 10:53, Qu Wenruo 写道:
> With the recent *_compressed_bio() rework, we're using compressed_bio
> everywhere for reads/writes (including regular and encoded).
> 
> However this requires us to allocate/free folios using
> btrfs_alloc_compr_folio() and btrfs_free_compr_folio().

Sorry, this is only a cosmetic fix.

The btrfs_alloc_compr_folio() will only grab a folio from the 
compr_pool, and after that the folio can be handled just like a regular one.

It's recommended to use btrfs_free_compr_folio() so that pool can be 
re-charged, but not strictly necessary.

And even if exhausted the pool, we will just fallback to regular allocation.
Even without exhaustion, the pool can still be fully reclaimed when the 
shrinker is involved.

So this series is not addressing the problem that David is hitting.

I'm afraid the best way is just to get rid of the compr_pool completely.

Thanks,
Qu

> 
> Not using the later to release a folio allocated by
> btrfs_alloc_compr_folio() can screw up the LRU list.
> 
> Fix the unpaired calls as a hot fix.
> 
> For the long run, I'd like to remove btrfs_(alloc|free)_compr_folios()
> with regular folio_(alloc|put)() instead.
> As I do not think the compr pool is that helpful compared to the extra
> maintanance needed.
> 
> Qu Wenruo (2):
>    btrfs: fix the incorrect freeing of a compression folio in
>      btrfs_submit_compressed_read()
>    btrfs: fix the incorrect freeing of a compression folio in
>      btrfs_do_encoded_write()
> 
>   fs/btrfs/compression.c | 2 +-
>   fs/btrfs/inode.c       | 4 ++--
>   2 files changed, 3 insertions(+), 3 deletions(-)
>