[PATCH v4] btrfs: retry verity reads for not-uptodate Merkle folios

[PATCH v4] btrfs: retry verity reads for not-uptodate Merkle folios

[Yichong Chen]

btrfs_read_merkle_tree_page() can find a folio in the mapping that is not
uptodate.  After taking the folio lock, the current code treats that state
as a read error and returns -EIO.

That can make a previous transient read failure sticky.  If the failed read
left a not-uptodate folio in the mapping, later callers find that folio and
fail instead of retrying the read.

Keep the existing page-cache insertion and locking order, but retry the
Merkle item read when a not-uptodate folio is found in the mapping.  Also
unlock the folio when read_key_bytes() fails so that a later caller can
lock it and retry the read.

Fixes: 06ed09351b67 ("btrfs: convert btrfs_read_merkle_tree_page() to use a folio")
Reviewed-by: Boris Burkov <boris@bur.io>
Signed-off-by: Yichong Chen <chenyichong@uniontech.com>
---
v4:
- Add a comment explaining the locked uptodate recheck.
- Add Boris' Reviewed-by.

v3:
- Keep the existing filemap_add_folio() and read ordering.
- Retry the Merkle item read when a not-uptodate folio is found, as
  suggested by Boris.
- Unlock the folio on read_key_bytes() failure so later callers can retry.

v2:
- Avoid calling filemap_remove_folio(), which is not exported.
- Add the folio to the page cache only after read_key_bytes() succeeds.
---
 fs/btrfs/verity.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/verity.c b/fs/btrfs/verity.c
index 983365a73541..1133a56c0568 100644
--- a/fs/btrfs/verity.c
+++ b/fs/btrfs/verity.c
@@ -720,14 +720,18 @@ static struct page *btrfs_read_merkle_tree_page(struct inode *inode,
 			goto out;
 
 		folio_lock(folio);
-		/* If it's not uptodate after we have the lock, we got a read error. */
-		if (!folio_test_uptodate(folio)) {
+		/* Folio was truncated from mapping. */
+		if (!folio->mapping) {
 			folio_unlock(folio);
 			folio_put(folio);
-			return ERR_PTR(-EIO);
+			goto again;
 		}
-		folio_unlock(folio);
-		goto out;
+		/* Another reader may have filled the folio while we waited. */
+		if (folio_test_uptodate(folio)) {
+			folio_unlock(folio);
+			goto out;
+		}
+		goto read_folio;
 	}
 
 	folio = filemap_alloc_folio(mapping_gfp_constraint(inode->i_mapping, ~__GFP_FS),
@@ -744,6 +748,7 @@ static struct page *btrfs_read_merkle_tree_page(struct inode *inode,
 		return ERR_PTR(ret);
 	}
 
+read_folio:
 	/*
 	 * Merkle item keys are indexed from byte 0 in the merkle tree.
 	 * They have the form:
@@ -753,6 +758,7 @@ static struct page *btrfs_read_merkle_tree_page(struct inode *inode,
 	ret = read_key_bytes(BTRFS_I(inode), BTRFS_VERITY_MERKLE_ITEM_KEY, off,
 			     folio_address(folio), PAGE_SIZE, folio);
 	if (ret < 0) {
+		folio_unlock(folio);
 		folio_put(folio);
 		return ERR_PTR(ret);
 	}
-- 
2.51.0

Re: [PATCH v4] btrfs: retry verity reads for not-uptodate Merkle folios

[David Sterba]

On Wed, Jul 22, 2026 at 10:54:35AM +0800, Yichong Chen wrote:
> btrfs_read_merkle_tree_page() can find a folio in the mapping that is not
> uptodate.  After taking the folio lock, the current code treats that state
> as a read error and returns -EIO.
> 
> That can make a previous transient read failure sticky.  If the failed read
> left a not-uptodate folio in the mapping, later callers find that folio and
> fail instead of retrying the read.
> 
> Keep the existing page-cache insertion and locking order, but retry the
> Merkle item read when a not-uptodate folio is found in the mapping.  Also
> unlock the folio when read_key_bytes() fails so that a later caller can
> lock it and retry the read.
> 
> Fixes: 06ed09351b67 ("btrfs: convert btrfs_read_merkle_tree_page() to use a folio")
> Reviewed-by: Boris Burkov <boris@bur.io>
> Signed-off-by: Yichong Chen <chenyichong@uniontech.com>
> ---
> v4:
> - Add a comment explaining the locked uptodate recheck.
> - Add Boris' Reviewed-by.
> 
> v3:
> - Keep the existing filemap_add_folio() and read ordering.
> - Retry the Merkle item read when a not-uptodate folio is found, as
>   suggested by Boris.
> - Unlock the folio on read_key_bytes() failure so later callers can retry.
> 
> v2:
> - Avoid calling filemap_remove_folio(), which is not exported.
> - Add the folio to the page cache only after read_key_bytes() succeeds.
> ---
>  fs/btrfs/verity.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/btrfs/verity.c b/fs/btrfs/verity.c
> index 983365a73541..1133a56c0568 100644
> --- a/fs/btrfs/verity.c
> +++ b/fs/btrfs/verity.c
> @@ -720,14 +720,18 @@ static struct page *btrfs_read_merkle_tree_page(struct inode *inode,
>  			goto out;
>  
>  		folio_lock(folio);
> -		/* If it's not uptodate after we have the lock, we got a read error. */
> -		if (!folio_test_uptodate(folio)) {
> +		/* Folio was truncated from mapping. */
> +		if (!folio->mapping) {
>  			folio_unlock(folio);
>  			folio_put(folio);
> -			return ERR_PTR(-EIO);
> +			goto again;
>  		}
> -		folio_unlock(folio);
> -		goto out;
> +		/* Another reader may have filled the folio while we waited. */
> +		if (folio_test_uptodate(folio)) {
> +			folio_unlock(folio);
> +			goto out;
> +		}
> +		goto read_folio;

With that new label the main loop becomes a less readable, previously
there was just again and out. From a distance the loop likes a big while
from again: to read_key_bytes, so this may be a good followup cleanup
(if the end result is reasonable).

>  	}
>  
>  	folio = filemap_alloc_folio(mapping_gfp_constraint(inode->i_mapping, ~__GFP_FS),
> @@ -744,6 +748,7 @@ static struct page *btrfs_read_merkle_tree_page(struct inode *inode,
>  		return ERR_PTR(ret);
>  	}
>  
> +read_folio:
>  	/*
>  	 * Merkle item keys are indexed from byte 0 in the merkle tree.
>  	 * They have the form:
> @@ -753,6 +758,7 @@ static struct page *btrfs_read_merkle_tree_page(struct inode *inode,
>  	ret = read_key_bytes(BTRFS_I(inode), BTRFS_VERITY_MERKLE_ITEM_KEY, off,
>  			     folio_address(folio), PAGE_SIZE, folio);
>  	if (ret < 0) {
> +		folio_unlock(folio);
>  		folio_put(folio);
>  		return ERR_PTR(ret);
>  	}

Re: [PATCH v4] btrfs: retry verity reads for not-uptodate Merkle folios

[Matthew Wilcox]

On Thu, Jul 23, 2026 at 05:14:44AM +0200, David Sterba wrote:
> With that new label the main loop becomes a less readable, previously
> there was just again and out. From a distance the loop likes a big while
> from again: to read_key_bytes, so this may be a good followup cleanup
> (if the end result is reasonable).

I think a better followup patch would be to use read_cache_folio()
(or similar) rather then duplicating that logic in btrfs ...

Re: [PATCH v4] btrfs: retry verity reads for not-uptodate Merkle folios

[David Sterba]

On Wed, Jul 22, 2026 at 10:54:35AM +0800, Yichong Chen wrote:
> btrfs_read_merkle_tree_page() can find a folio in the mapping that is not
> uptodate.  After taking the folio lock, the current code treats that state
> as a read error and returns -EIO.
> 
> That can make a previous transient read failure sticky.  If the failed read
> left a not-uptodate folio in the mapping, later callers find that folio and
> fail instead of retrying the read.
> 
> Keep the existing page-cache insertion and locking order, but retry the
> Merkle item read when a not-uptodate folio is found in the mapping.  Also
> unlock the folio when read_key_bytes() fails so that a later caller can
> lock it and retry the read.
> 
> Fixes: 06ed09351b67 ("btrfs: convert btrfs_read_merkle_tree_page() to use a folio")
> Reviewed-by: Boris Burkov <boris@bur.io>
> Signed-off-by: Yichong Chen <chenyichong@uniontech.com>

Added to for-next, thanks.

Re: [PATCH v4] btrfs: retry verity reads for not-uptodate Merkle folios

[Matthew Wilcox]

On Wed, Jul 22, 2026 at 10:54:35AM +0800, Yichong Chen wrote:
> btrfs_read_merkle_tree_page() can find a folio in the mapping that is not
> uptodate.  After taking the folio lock, the current code treats that state
> as a read error and returns -EIO.
> 
> That can make a previous transient read failure sticky.  If the failed read
> left a not-uptodate folio in the mapping, later callers find that folio and
> fail instead of retrying the read.
> 
> Keep the existing page-cache insertion and locking order, but retry the
> Merkle item read when a not-uptodate folio is found in the mapping.  Also
> unlock the folio when read_key_bytes() fails so that a later caller can
> lock it and retry the read.
> 
> Fixes: 06ed09351b67 ("btrfs: convert btrfs_read_merkle_tree_page() to use a folio")

I object to this Fixes line.  I didn't introduce this problem; it was
already there before 06ed09351b67.

Re: [PATCH v4] btrfs: retry verity reads for not-uptodate Merkle folios

[David Sterba]

On Mon, Aug 17, 2026 at 04:05:08AM +0100, Matthew Wilcox wrote:
> On Wed, Jul 22, 2026 at 10:54:35AM +0800, Yichong Chen wrote:
> > btrfs_read_merkle_tree_page() can find a folio in the mapping that is not
> > uptodate.  After taking the folio lock, the current code treats that state
> > as a read error and returns -EIO.
> > 
> > That can make a previous transient read failure sticky.  If the failed read
> > left a not-uptodate folio in the mapping, later callers find that folio and
> > fail instead of retrying the read.
> > 
> > Keep the existing page-cache insertion and locking order, but retry the
> > Merkle item read when a not-uptodate folio is found in the mapping.  Also
> > unlock the folio when read_key_bytes() fails so that a later caller can
> > lock it and retry the read.
> > 
> > Fixes: 06ed09351b67 ("btrfs: convert btrfs_read_merkle_tree_page() to use a folio")
> 
> I object to this Fixes line.  I didn't introduce this problem; it was
> already there before 06ed09351b67.

You're right, however we need to keep the patch as-is because that would
require a rebase and delay sending the main 7.3 pull request, it's below
22 patches.