[PATCH] erofs: fix missing folio_unlock causing lock imbalance

[PATCH] erofs: fix missing folio_unlock causing lock imbalance

[Zhan Xusheng]

folio_trylock() in erofs_try_to_free_all_cached_folios() may
successfully acquire the folio lock, but the subsequent check
for erofs_folio_is_managed() can skip unlocking when the folio
is not managed by EROFS.

This leads to a lock imbalance and leaves the folio permanently
locked, which may cause reclaim stalls or interfere with memory
management.

Fix this by ensuring folio_unlock() is called before continuing.

Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
 fs/erofs/zdata.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
index fe8121df9ef2..9d7ff22f1622 100644
--- a/fs/erofs/zdata.c
+++ b/fs/erofs/zdata.c
@@ -605,8 +605,10 @@ static int erofs_try_to_free_all_cached_folios(struct erofs_sb_info *sbi,
 			if (!folio_trylock(folio))
 				return -EBUSY;
 
-			if (!erofs_folio_is_managed(sbi, folio))
+			if (!erofs_folio_is_managed(sbi, folio)) {
+				folio_unlock(folio);
 				continue;
+			}
 			pcl->compressed_bvecs[i].page = NULL;
 			folio_detach_private(folio);
 			folio_unlock(folio);
-- 
2.43.0

Re: [PATCH] erofs: fix missing folio_unlock causing lock imbalance

[Gao Xiang]

Hi Zhan,

On 2026/3/31 10:33, Zhan Xusheng wrote:
> folio_trylock() in erofs_try_to_free_all_cached_folios() may
> successfully acquire the folio lock, but the subsequent check
> for erofs_folio_is_managed() can skip unlocking when the folio
> is not managed by EROFS.

Do you find some real timing?

I don't think it can really happen, because:

> 
> This leads to a lock imbalance and leaves the folio permanently
> locked, which may cause reclaim stalls or interfere with memory
> management.
> 
> Fix this by ensuring folio_unlock() is called before continuing.

If a folio links to a pcluster, folio->private will be non-NULL,
and pcl->compressed_bvecs[i] points to that folios.

And z_erofs_cache_release_folio() will be called with folio lock,
and pcl->compressed_bvecs[i] will be set NULL here.

So I don't think erofs_try_to_free_all_cached_folios() can find
!erofs_folio_is_managed(sbi, folio) in the real world.

> 
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> ---
>   fs/erofs/zdata.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
> index fe8121df9ef2..9d7ff22f1622 100644
> --- a/fs/erofs/zdata.c
> +++ b/fs/erofs/zdata.c
> @@ -605,8 +605,10 @@ static int erofs_try_to_free_all_cached_folios(struct erofs_sb_info *sbi,
>   			if (!folio_trylock(folio))
>   				return -EBUSY;
>   
> -			if (!erofs_folio_is_managed(sbi, folio))
> +			if (!erofs_folio_is_managed(sbi, folio)) {
> +				folio_unlock(folio);
>   				continue;
> +			}
>   			pcl->compressed_bvecs[i].page = NULL;
>   			folio_detach_private(folio);

But I admit that we should rewrite in function as:

			if (!erofs_folio_is_managed(sbi, folio)) {
				DBG_BUGON(1);
			} else {
				pcl->compressed_bvecs[i].page = NULL;
				folio_detach_private(folio);
			}
			folio_unlock(folio);

Thanks,
Gao Xiang


>   			folio_unlock(folio);

Re: [PATCH] erofs: fix missing folio_unlock causing lock imbalance

[Gao Xiang]

On 2026/3/31 10:50, Gao Xiang wrote:
> Hi Zhan,
> 
> On 2026/3/31 10:33, Zhan Xusheng wrote:
>> folio_trylock() in erofs_try_to_free_all_cached_folios() may
>> successfully acquire the folio lock, but the subsequent check
>> for erofs_folio_is_managed() can skip unlocking when the folio
>> is not managed by EROFS.
> 
> Do you find some real timing?
> 
> I don't think it can really happen, because:
> 
>>
>> This leads to a lock imbalance and leaves the folio permanently
>> locked, which may cause reclaim stalls or interfere with memory
>> management.
>>
>> Fix this by ensuring folio_unlock() is called before continuing.
> 
> If a folio links to a pcluster, folio->private will be non-NULL,
> and pcl->compressed_bvecs[i] points to that folios.
> 
> And z_erofs_cache_release_folio() will be called with folio lock,
> and pcl->compressed_bvecs[i] will be set NULL here.
> 
> So I don't think erofs_try_to_free_all_cached_folios() can find
> !erofs_folio_is_managed(sbi, folio) in the real world.
> 
>>
>> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
>> ---
>>   fs/erofs/zdata.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
>> index fe8121df9ef2..9d7ff22f1622 100644
>> --- a/fs/erofs/zdata.c
>> +++ b/fs/erofs/zdata.c
>> @@ -605,8 +605,10 @@ static int erofs_try_to_free_all_cached_folios(struct erofs_sb_info *sbi,
>>               if (!folio_trylock(folio))
>>                   return -EBUSY;
>> -            if (!erofs_folio_is_managed(sbi, folio))
>> +            if (!erofs_folio_is_managed(sbi, folio)) {
>> +                folio_unlock(folio);
>>                   continue;
>> +            }
>>               pcl->compressed_bvecs[i].page = NULL;
>>               folio_detach_private(folio);
> 
> But I admit that we should rewrite in function as:
> 
>              if (!erofs_folio_is_managed(sbi, folio)) {
>                  DBG_BUGON(1);
>              } else {
>                  pcl->compressed_bvecs[i].page = NULL;
>                  folio_detach_private(folio);
>              }

Or maybe just:
		DBG_BUGON(!erofs_folio_is_managed(sbi, folio));
		pcl->compressed_bvecs[i].page = NULL;
		folio_detach_private(folio);
		folio_unlock(folio);

Since if a pcluster goes here (!pcl->lockref.count),
`pcl->compressed_bvecs[i]` should leave all valid cached
folios (Or some should be recycled by .release_folio
instead.)

Unless there is the other bug somewhere, but in any case,
I don't think your phenomenon is related to EROFS.

Thanks,
Gao Xiang

>              folio_unlock(folio);
> 
> Thanks,
> Gao Xiang
> 
> 
>>               folio_unlock(folio);
> 

Re: [PATCH] erofs: fix missing folio_unlock causing lock imbalance

[Zhan Xusheng]

Hi Gao,

Thanks for the detailed explanation.

I see your point that folios attached to a pcluster should
always be managed by EROFS, and the !erofs_folio_is_managed()
case should not happen in practice.

Instead of handling it silently, it makes more sense to
treat this as an invariant and catch unexpected cases
explicitly.

I'll update the patch accordingly by adding DBG_BUGON()
and send a v2.

Thanks,
Zhan Xusheng

[PATCH] [PATCH v2] erofs: fix missing folio_unlock causing lock imbalance

[Zhan Xusheng]

folio_trylock() in erofs_try_to_free_all_cached_folios() may
successfully acquire the folio lock, but the subsequent check
for erofs_folio_is_managed() can skip unlocking when the folio
is not managed by EROFS.

As Gao Xiang pointed out, this condition should not happen in
practice because compressed_bvecs[] only holds valid cached folios
at this point — any non-managed folio would have already been
detached by z_erofs_cache_release_folio() under folio lock.

Fix this by adding DBG_BUGON() to catch unexpected folios
and ensure folio_unlock() is always called.

Suggested-by: Gao Xiang <hsiangkao@linux.alibaba.com>
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
 fs/erofs/zdata.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
index fe8121df9ef2..b566996a0d1a 100644
--- a/fs/erofs/zdata.c
+++ b/fs/erofs/zdata.c
@@ -605,8 +605,7 @@ static int erofs_try_to_free_all_cached_folios(struct erofs_sb_info *sbi,
 			if (!folio_trylock(folio))
 				return -EBUSY;
 
-			if (!erofs_folio_is_managed(sbi, folio))
-				continue;
+			DBG_BUGON(!erofs_folio_is_managed(sbi, folio));
 			pcl->compressed_bvecs[i].page = NULL;
 			folio_detach_private(folio);
 			folio_unlock(folio);
-- 
2.43.0

Re: [PATCH] [PATCH v2] erofs: fix missing folio_unlock causing lock imbalance

[Gao Xiang]

On 2026/3/31 11:46, Zhan Xusheng wrote:
> folio_trylock() in erofs_try_to_free_all_cached_folios() may
> successfully acquire the folio lock, but the subsequent check
> for erofs_folio_is_managed() can skip unlocking when the folio
> is not managed by EROFS.
> 
> As Gao Xiang pointed out, this condition should not happen in
> practice because compressed_bvecs[] only holds valid cached folios
> at this point — any non-managed folio would have already been
> detached by z_erofs_cache_release_folio() under folio lock.
> 
> Fix this by adding DBG_BUGON() to catch unexpected folios
> and ensure folio_unlock() is always called.
> 
> Suggested-by: Gao Xiang <hsiangkao@linux.alibaba.com>
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>

emmm, the patch subject needs to be changed too, like:

[PATCH v3] erofs: ensure all folios are managed in erofs_try_to_free_all_cached_folios()

Thanks,
Gao Xiang

> ---
>   fs/erofs/zdata.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
> index fe8121df9ef2..b566996a0d1a 100644
> --- a/fs/erofs/zdata.c
> +++ b/fs/erofs/zdata.c
> @@ -605,8 +605,7 @@ static int erofs_try_to_free_all_cached_folios(struct erofs_sb_info *sbi,
>   			if (!folio_trylock(folio))
>   				return -EBUSY;
>   
> -			if (!erofs_folio_is_managed(sbi, folio))
> -				continue;
> +			DBG_BUGON(!erofs_folio_is_managed(sbi, folio));
>   			pcl->compressed_bvecs[i].page = NULL;
>   			folio_detach_private(folio);
>   			folio_unlock(folio);

[PATCH] [PATCH v3] erofs: ensure all folios are managed in erofs_try_to_free_all_cached_folios()

[Zhan Xusheng]

folio_trylock() in erofs_try_to_free_all_cached_folios() may
successfully acquire the folio lock, but the subsequent check
for erofs_folio_is_managed() can skip unlocking when the folio
is not managed by EROFS.

As Gao Xiang pointed out, this condition should not happen in
practice because compressed_bvecs[] only holds valid cached folios
at this point — any non-managed folio would have already been
detached by z_erofs_cache_release_folio() under folio lock.

Fix this by adding DBG_BUGON() to catch unexpected folios
and ensure folio_unlock() is always called.

Suggested-by: Gao Xiang <hsiangkao@linux.alibaba.com>
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
 fs/erofs/zdata.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/erofs/zdata.c b/fs/erofs/zdata.c
index fe8121df9ef2..b566996a0d1a 100644
--- a/fs/erofs/zdata.c
+++ b/fs/erofs/zdata.c
@@ -605,8 +605,7 @@ static int erofs_try_to_free_all_cached_folios(struct erofs_sb_info *sbi,
 			if (!folio_trylock(folio))
 				return -EBUSY;
 
-			if (!erofs_folio_is_managed(sbi, folio))
-				continue;
+			DBG_BUGON(!erofs_folio_is_managed(sbi, folio));
 			pcl->compressed_bvecs[i].page = NULL;
 			folio_detach_private(folio);
 			folio_unlock(folio);
-- 
2.43.0

Re: [PATCH] [PATCH v3] erofs: ensure all folios are managed in erofs_try_to_free_all_cached_folios()

[Gao Xiang]

On 2026/3/31 13:02, Zhan Xusheng wrote:
> folio_trylock() in erofs_try_to_free_all_cached_folios() may
> successfully acquire the folio lock, but the subsequent check
> for erofs_folio_is_managed() can skip unlocking when the folio
> is not managed by EROFS.
> 
> As Gao Xiang pointed out, this condition should not happen in
> practice because compressed_bvecs[] only holds valid cached folios
> at this point — any non-managed folio would have already been
> detached by z_erofs_cache_release_folio() under folio lock.
> 
> Fix this by adding DBG_BUGON() to catch unexpected folios
> and ensure folio_unlock() is always called.
> 
> Suggested-by: Gao Xiang <hsiangkao@linux.alibaba.com>
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>

Reviewed-by: Gao Xiang <hsiangkao@linux.alibaba.com>

Thanks,
Gao Xiang

Re: [PATCH] [PATCH v3] erofs: ensure all folios are managed in erofs_try_to_free_all_cached_folios()

[Chunhai Guo]

On 3/31/2026 1:02 PM, Zhan Xusheng wrote:
> folio_trylock() in erofs_try_to_free_all_cached_folios() may
> successfully acquire the folio lock, but the subsequent check
> for erofs_folio_is_managed() can skip unlocking when the folio
> is not managed by EROFS.
>
> As Gao Xiang pointed out, this condition should not happen in
> practice because compressed_bvecs[] only holds valid cached folios
> at this point — any non-managed folio would have already been
> detached by z_erofs_cache_release_folio() under folio lock.
>
> Fix this by adding DBG_BUGON() to catch unexpected folios
> and ensure folio_unlock() is always called.
>
> Suggested-by: Gao Xiang <hsiangkao@linux.alibaba.com>
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>

Reviewed-by: Chunhai Guo <guochunhai@vivo.com>


Thanks,