[PATCH] btrfs: remove the folio ref count ASSERT() from btrfs_free_comp_folio()

[PATCH] btrfs: remove the folio ref count ASSERT() from btrfs_free_comp_folio()

[Qu Wenruo]

Inside btrfs_free_compr_folio() we have an ASSERT() to make sure when we
free the folio it should only have one reference (by btrfs).

However there is never any guarantee that btrfs is the only holder of
the folio. Memory management may have acquired that folio for whatever
reasons.

I do not think we should poke into the very low-level implementation
of memory management code, and I didn't find any fs code really using
folio_ref_count() other than during debugging output.

Just remove the ASSERT() to avoid false triggering.

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

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index e897342bece1..192f133d9eb5 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -224,7 +224,6 @@ void btrfs_free_compr_folio(struct folio *folio)
 		return;
 
 free:
-	ASSERT(folio_ref_count(folio) == 1);
 	folio_put(folio);
 }
 
-- 
2.53.0

Re: [PATCH] btrfs: remove the folio ref count ASSERT() from btrfs_free_comp_folio()

[Boris Burkov]

On Thu, Feb 26, 2026 at 10:50:38AM +1030, Qu Wenruo wrote:
> Inside btrfs_free_compr_folio() we have an ASSERT() to make sure when we
> free the folio it should only have one reference (by btrfs).
> 
> However there is never any guarantee that btrfs is the only holder of
> the folio. Memory management may have acquired that folio for whatever
> reasons.
> 
> I do not think we should poke into the very low-level implementation
> of memory management code, and I didn't find any fs code really using
> folio_ref_count() other than during debugging output.
> 
> Just remove the ASSERT() to avoid false triggering.
> 

Reviewed-by: Boris Burkov <boris@bur.io>

> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/compression.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
> index e897342bece1..192f133d9eb5 100644
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -224,7 +224,6 @@ void btrfs_free_compr_folio(struct folio *folio)
>  		return;
>  
>  free:
> -	ASSERT(folio_ref_count(folio) == 1);
>  	folio_put(folio);
>  }
>  
> -- 
> 2.53.0
> 

Re: [PATCH] btrfs: remove the folio ref count ASSERT() from btrfs_free_comp_folio()

[Johannes Thumshirn]

On 2/26/26 1:21 AM, Qu Wenruo wrote:
> Inside btrfs_free_compr_folio() we have an ASSERT() to make sure when we
> free the folio it should only have one reference (by btrfs).
>
> However there is never any guarantee that btrfs is the only holder of
> the folio. Memory management may have acquired that folio for whatever
> reasons.
>
> I do not think we should poke into the very low-level implementation
> of memory management code, and I didn't find any fs code really using
> folio_ref_count() other than during debugging output.
>
> Just remove the ASSERT() to avoid false triggering.
>
Just curious, did this trigger any bug or did you find it by coincidence?


Anyways,

Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Re: [PATCH] btrfs: remove the folio ref count ASSERT() from btrfs_free_comp_folio()

[Qu Wenruo]

在 2026/2/26 17:13, Johannes Thumshirn 写道:
> On 2/26/26 1:21 AM, Qu Wenruo wrote:
>> Inside btrfs_free_compr_folio() we have an ASSERT() to make sure when we
>> free the folio it should only have one reference (by btrfs).
>>
>> However there is never any guarantee that btrfs is the only holder of
>> the folio. Memory management may have acquired that folio for whatever
>> reasons.
>>
>> I do not think we should poke into the very low-level implementation
>> of memory management code, and I didn't find any fs code really using
>> folio_ref_count() other than during debugging output.
>>
>> Just remove the ASSERT() to avoid false triggering.
>>
> Just curious, did this trigger any bug or did you find it by coincidence?

David found one, and I failed to trigger it.

The deeper problem here is not the ASSERT() itself, it's the ref count 
reached 0 already before we call folio_put().

In that case, even without our ASSERT(), VM_BUG_ON() will still be 
triggered.

Thanks,
Qu

> 
> 
> Anyways,
> 
> Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> 

Re: [PATCH] btrfs: remove the folio ref count ASSERT() from btrfs_free_comp_folio()

[David Sterba]

On Thu, Feb 26, 2026 at 06:43:07AM +0000, Johannes Thumshirn wrote:
> On 2/26/26 1:21 AM, Qu Wenruo wrote:
> > Inside btrfs_free_compr_folio() we have an ASSERT() to make sure when we
> > free the folio it should only have one reference (by btrfs).
> >
> > However there is never any guarantee that btrfs is the only holder of
> > the folio. Memory management may have acquired that folio for whatever
> > reasons.
> >
> > I do not think we should poke into the very low-level implementation
> > of memory management code, and I didn't find any fs code really using
> > folio_ref_count() other than during debugging output.
> >
> > Just remove the ASSERT() to avoid false triggering.
> >
> Just curious, did this trigger any bug or did you find it by coincidence?

With other tests' workload it was triggered by btrfs/011 on a machine
with 1G of memory.