[PATCH] btrfs: remove verity Merkle folio from page cache on error

[PATCH] btrfs: remove verity Merkle folio from page cache on error

[Yichong Chen]

btrfs_read_merkle_tree_page() allocates a folio and adds it to the
page cache before reading the Merkle tree data from btree items.

If read_key_bytes() fails, the folio is still locked and present in the
page cache. Drop it from the page cache and unlock it before releasing the
allocation reference so a later read can retry instead of finding a locked,
not-uptodate folio.

Fixes: 06ed09351b67 ("btrfs: convert btrfs_read_merkle_tree_page() to use a folio")
Signed-off-by: Yichong Chen <chenyichong@uniontech.com>
---
 fs/btrfs/verity.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/btrfs/verity.c b/fs/btrfs/verity.c
index 983365a73541..99468791a98e 100644
--- a/fs/btrfs/verity.c
+++ b/fs/btrfs/verity.c
@@ -753,6 +753,8 @@ 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) {
+		filemap_remove_folio(folio);
+		folio_unlock(folio);
 		folio_put(folio);
 		return ERR_PTR(ret);
 	}
-- 
2.51.0

Re: [PATCH] btrfs: remove verity Merkle folio from page cache on error

[Yichong Chen]

Hi,

Please ignore this version.

I noticed that this version depends on filemap_remove_folio(). My local
debug tree had this symbol exported, so the patch built there, but exporting
that helper does not look like the right approach for this fix.

I will rework the fix to avoid inserting the folio into the page cache
before the Merkle tree read succeeds, and send a v2.

Sorry for the noise.

Thanks,
Yichong

[PATCH v2] btrfs: add verity Merkle folio to page cache after reading it

[Yichong Chen]

btrfs_read_merkle_tree_page() allocates a folio and adds it to the page
cache before reading the Merkle tree data from btree items.

If read_key_bytes() fails, the folio is still locked and present in the
page cache.  A later read can find the locked, not-uptodate folio instead
of retrying the read.

Avoid installing the folio in the page cache until after the Merkle tree
read has succeeded.  This keeps the failure path simple: the newly
allocated folio is not visible to the page cache yet and only needs to be
released.

Fixes: 06ed09351b67 ("btrfs: convert btrfs_read_merkle_tree_page() to use a folio")
Signed-off-by: Yichong Chen <chenyichong@uniontech.com>
---
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 | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/fs/btrfs/verity.c b/fs/btrfs/verity.c
index 983365a73541..d705ce3b386a 100644
--- a/fs/btrfs/verity.c
+++ b/fs/btrfs/verity.c
@@ -735,15 +735,6 @@ static struct page *btrfs_read_merkle_tree_page(struct inode *inode,
 	if (!folio)
 		return ERR_PTR(-ENOMEM);
 
-	ret = filemap_add_folio(inode->i_mapping, folio, index, GFP_NOFS);
-	if (ret) {
-		folio_put(folio);
-		/* Did someone else insert a folio here? */
-		if (ret == -EEXIST)
-			goto again;
-		return ERR_PTR(ret);
-	}
-
 	/*
 	 * Merkle item keys are indexed from byte 0 in the merkle tree.
 	 * They have the form:
@@ -759,6 +750,15 @@ static struct page *btrfs_read_merkle_tree_page(struct inode *inode,
 	if (ret < PAGE_SIZE)
 		folio_zero_segment(folio, ret, PAGE_SIZE);
 
+	ret = filemap_add_folio(inode->i_mapping, folio, index, GFP_NOFS);
+	if (ret) {
+		folio_put(folio);
+		/* Did someone else insert a folio here? */
+		if (ret == -EEXIST)
+			goto again;
+		return ERR_PTR(ret);
+	}
+
 	folio_mark_uptodate(folio);
 	folio_unlock(folio);
 
-- 
2.51.0

Re: [PATCH v2] btrfs: add verity Merkle folio to page cache after reading it

[Boris Burkov]

On Tue, Jul 07, 2026 at 01:55:17PM +0800, Yichong Chen wrote:
> btrfs_read_merkle_tree_page() allocates a folio and adds it to the page
> cache before reading the Merkle tree data from btree items.

So does ext4 via (eventually) do_read_cache_folio().

> 
> If read_key_bytes() fails, the folio is still locked and present in the
> page cache.  A later read can find the locked, not-uptodate folio instead
> of retrying the read.

I think the better fix is to fall through to retrying when we see a
not-uptodate folio in the mapping after __filemap_get_folio(), which I
believe is how the ext4 code will behave in that case.

> 
> Avoid installing the folio in the page cache until after the Merkle tree
> read has succeeded.  This keeps the failure path simple: the newly
> allocated folio is not visible to the page cache yet and only needs to be
> released.

I don't think this order is correct. filemap_add_folio() locks the folio
so if you do it in this order, you no longer lock the folio while
reading which is a non-trivial change. I don't immediately see what is
wrong with that off the top of my head for read only verity past eof
pages but it feels sloppy at the very least.

Thanks,
Boris

> 
> Fixes: 06ed09351b67 ("btrfs: convert btrfs_read_merkle_tree_page() to use a folio")
> Signed-off-by: Yichong Chen <chenyichong@uniontech.com>
> ---
> 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 | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/btrfs/verity.c b/fs/btrfs/verity.c
> index 983365a73541..d705ce3b386a 100644
> --- a/fs/btrfs/verity.c
> +++ b/fs/btrfs/verity.c
> @@ -735,15 +735,6 @@ static struct page *btrfs_read_merkle_tree_page(struct inode *inode,
>  	if (!folio)
>  		return ERR_PTR(-ENOMEM);
>  
> -	ret = filemap_add_folio(inode->i_mapping, folio, index, GFP_NOFS);
> -	if (ret) {
> -		folio_put(folio);
> -		/* Did someone else insert a folio here? */
> -		if (ret == -EEXIST)
> -			goto again;
> -		return ERR_PTR(ret);
> -	}
> -
>  	/*
>  	 * Merkle item keys are indexed from byte 0 in the merkle tree.
>  	 * They have the form:
> @@ -759,6 +750,15 @@ static struct page *btrfs_read_merkle_tree_page(struct inode *inode,
>  	if (ret < PAGE_SIZE)
>  		folio_zero_segment(folio, ret, PAGE_SIZE);
>  
> +	ret = filemap_add_folio(inode->i_mapping, folio, index, GFP_NOFS);
> +	if (ret) {
> +		folio_put(folio);
> +		/* Did someone else insert a folio here? */
> +		if (ret == -EEXIST)
> +			goto again;
> +		return ERR_PTR(ret);
> +	}
> +
>  	folio_mark_uptodate(folio);
>  	folio_unlock(folio);
>  
> -- 
> 2.51.0

Re: [PATCH v2] btrfs: add verity Merkle folio to page cache after reading it

[Yichong Chen]

Thanks for the review.

I agree.  I'll rework it to keep the existing page-cache insertion and
locking order, and handle the not-uptodate folio case by retrying the
read as suggested.

Thanks,
Yichong

Re: [PATCH] btrfs: remove verity Merkle folio from page cache on error

[kernel test robot]

Hi Yichong,

kernel test robot noticed the following build errors:

[auto build test ERROR on linus/master]
[also build test ERROR on v7.2-rc7]
[cannot apply to kdave/for-next next-20260813]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Yichong-Chen/btrfs-remove-verity-Merkle-folio-from-page-cache-on-error/20260813-080149
base:   linus/master
patch link:    https://lore.kernel.org/r/20260706053907.544337-1-chenyichong%40uniontech.com
patch subject: [PATCH] btrfs: remove verity Merkle folio from page cache on error
config: nios2-allmodconfig (https://download.01.org/0day-ci/archive/20260815/202608150828.VDr6P7DT-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 11.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260815/202608150828.VDr6P7DT-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202608150828.VDr6P7DT-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "filemap_remove_folio" [fs/btrfs/btrfs.ko] undefined!

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] btrfs: remove verity Merkle folio from page cache on error

[Yichong Chen]

Hi,

Thanks for the report.

This was a problem in v1.  The patch called filemap_remove_folio(), which is
not exported and therefore breaks when btrfs is built as a module.

I avoided filemap_remove_folio() in v2, and the patch has since been reworked
further.  The latest version no longer calls filemap_remove_folio():

  [PATCH v4] btrfs: retry verity reads for not-uptodate Merkle folios
  https://lore.kernel.org/all/20260722025435.1493093-1-chenyichong@uniontech.com/

Thanks,
Yichong

Re: [PATCH] btrfs: remove verity Merkle folio from page cache on error

[Philip Li]

On Mon, Aug 17, 2026 at 10:37:26AM +0800, Yichong Chen wrote:
> Hi,
> 
> Thanks for the report.
> 
> This was a problem in v1.  The patch called filemap_remove_folio(), which is
> not exported and therefore breaks when btrfs is built as a module.
> 
> I avoided filemap_remove_folio() in v2, and the patch has since been reworked
> further.  The latest version no longer calls filemap_remove_folio():

Thanks for the info, and sorry for the meaningless report, we will improve
the logic to fix the bot.

> 
>   [PATCH v4] btrfs: retry verity reads for not-uptodate Merkle folios
>   https://lore.kernel.org/all/20260722025435.1493093-1-chenyichong@uniontech.com/
> 
> Thanks,
> Yichong
>