[PATCH v2] btrfs: handle lack of space when cleaning up verity items

[PATCH v2] btrfs: handle lack of space when cleaning up verity items

[Daniel Linjama]

When enable_verity() hits the qgroup limit, rollback_verity() needs its
own metadata reservation. When the qgroup limit or lack of space refuses
the rollback, the whole filesystem is forced read-only even though the
qgroup limit was for one subvolume only. Also orphan cleanup at the next
mount fails the same way, so the leftover items are never removed: with
-EDQUOT the subvolume stays unreachable, and with -ENOSPC on a full
filesystem the next read-write mount fails.

Start transactions with btrfs_start_transaction_fallback_global_rsv() in
btrfs_orphan_cleanup(), drop_verity_items() and rollback_verity() to
resolve the situation.

Fixes: 146054090b08 ("btrfs: initial fsverity support")
Signed-off-by: Daniel Linjama <daniel@dev.linjama.com>
---
Changes since v1:
- Revert the v1 change and fix the reservations on the cleanup path
  instead, as Qu suggested. Sashiko was right that v1 left the orphan
  for the next mount.
- Cover the three transactions on the rollback and cleanup path to
  handle -EDQUOT and -ENOSPC correctly.
- fstests: btrfs/354 (qgroup at its limit) and btrfs/355 (full
  filesystem), sent separately to fstests@vger.kernel.org:
  https://lore.kernel.org/fstests/20260915053815.307674-1-daniel@dev.linjama.com/

v1: https://lore.kernel.org/linux-btrfs/af99a1d5-25b0-4e39-9f66-a2dd8e1425c5@gmx.com/T/#t

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

diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 93ef3cec191e..d377f4ca5207 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -3877,7 +3877,8 @@ int btrfs_orphan_cleanup(struct btrfs_root *root)
 				if (ret)
 					goto out;
 			}
-			trans = btrfs_start_transaction(root, 1);
+			/* Only deletes the orphan, must not fail on a full qgroup */
+			trans = btrfs_start_transaction_fallback_global_rsv(root, 1);
 			if (IS_ERR(trans)) {
 				ret = PTR_ERR(trans);
 				goto out;
diff --git a/fs/btrfs/verity.c b/fs/btrfs/verity.c
index 4e0ab5842274..189b36e5fc29 100644
--- a/fs/btrfs/verity.c
+++ b/fs/btrfs/verity.c
@@ -119,8 +119,8 @@ static int drop_verity_items(struct btrfs_inode *inode, u8 key_type)
 		return -ENOMEM;
 
 	while (1) {
-		/* 1 for the item being dropped */
-		trans = btrfs_start_transaction(root, 1);
+		/* 1 for the item being dropped, must not fail on a full qgroup */
+		trans = btrfs_start_transaction_fallback_global_rsv(root, 1);
 		if (IS_ERR(trans))
 			return PTR_ERR(trans);
 
@@ -465,8 +465,11 @@ static int rollback_verity(struct btrfs_inode *inode)
 	/*
 	 * 1 for updating the inode flag
 	 * 1 for deleting the orphan
+	 *
+	 * Must not fail on a full qgroup either: the reservations for the
+	 * items dropped above are only released at commit.
 	 */
-	trans = btrfs_start_transaction(root, 2);
+	trans = btrfs_start_transaction_fallback_global_rsv(root, 2);
 	if (IS_ERR(trans)) {
 		ret = PTR_ERR(trans);
 		trans = NULL;

base-commit: 08df884136f1c1197bab2a27814404fd329d9aac
-- 
2.55.0

Re: [PATCH v2] btrfs: handle lack of space when cleaning up verity items

[Qu Wenruo]

在 2026/9/15 15:21, Daniel Linjama 写道:
> When enable_verity() hits the qgroup limit, rollback_verity() needs its
> own metadata reservation. When the qgroup limit or lack of space refuses
> the rollback, the whole filesystem is forced read-only even though the
> qgroup limit was for one subvolume only. Also orphan cleanup at the next
> mount fails the same way, so the leftover items are never removed: with
> -EDQUOT the subvolume stays unreachable, and with -ENOSPC on a full
> filesystem the next read-write mount fails.
> 
> Start transactions with btrfs_start_transaction_fallback_global_rsv() in
> btrfs_orphan_cleanup(), drop_verity_items() and rollback_verity() to
> resolve the situation.

The fix looks good to me, but please add some basic explanation on why 
we should use btrfs_start_transaction_fallback_global_rsv().

It's the same behavior as unlinking inode, we are dropping items which 
should release some space in the end, thus we're allowed to use global rsv.

The same also applies to the quota reservation part, we're deleting some 
items and should eventually free up some quota space, so here we should 
not be limited by qgroup limit.

> 
> Fixes: 146054090b08 ("btrfs: initial fsverity support")
> Signed-off-by: Daniel Linjama <daniel@dev.linjama.com>
> ---
> Changes since v1:
> - Revert the v1 change and fix the reservations on the cleanup path
>    instead, as Qu suggested. Sashiko was right that v1 left the orphan
>    for the next mount.
> - Cover the three transactions on the rollback and cleanup path to
>    handle -EDQUOT and -ENOSPC correctly.
> - fstests: btrfs/354 (qgroup at its limit) and btrfs/355 (full
>    filesystem), sent separately to fstests@vger.kernel.org:
>    https://lore.kernel.org/fstests/20260915053815.307674-1-daniel@dev.linjama.com/
> 
> v1: https://lore.kernel.org/linux-btrfs/af99a1d5-25b0-4e39-9f66-a2dd8e1425c5@gmx.com/T/#t
> 
>   fs/btrfs/inode.c  | 3 ++-
>   fs/btrfs/verity.c | 9 ++++++---
>   2 files changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 93ef3cec191e..d377f4ca5207 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -3877,7 +3877,8 @@ int btrfs_orphan_cleanup(struct btrfs_root *root)
>   				if (ret)
>   					goto out;
>   			}
> -			trans = btrfs_start_transaction(root, 1);
> +			/* Only deletes the orphan, must not fail on a full qgroup */

I think you may want to slightly change the comment, not only focusing 
on the qgroup part.

And to reduce duplication, you may want to follow __unlink_start_trans() 
to introduce a helper just to call 
btrfs_start_trans_fallback_global_rsv(), so that we only need one 
comment for the reason, without duplicating it for every caller.

Otherwise looks good to me now.

Thanks,
Qu

> +			trans = btrfs_start_transaction_fallback_global_rsv(root, 1);
>   			if (IS_ERR(trans)) {
>   				ret = PTR_ERR(trans);
>   				goto out;
> diff --git a/fs/btrfs/verity.c b/fs/btrfs/verity.c
> index 4e0ab5842274..189b36e5fc29 100644
> --- a/fs/btrfs/verity.c
> +++ b/fs/btrfs/verity.c
> @@ -119,8 +119,8 @@ static int drop_verity_items(struct btrfs_inode *inode, u8 key_type)
>   		return -ENOMEM;
>   
>   	while (1) {
> -		/* 1 for the item being dropped */
> -		trans = btrfs_start_transaction(root, 1);
> +		/* 1 for the item being dropped, must not fail on a full qgroup */
> +		trans = btrfs_start_transaction_fallback_global_rsv(root, 1);
>   		if (IS_ERR(trans))
>   			return PTR_ERR(trans);
>   
> @@ -465,8 +465,11 @@ static int rollback_verity(struct btrfs_inode *inode)
>   	/*
>   	 * 1 for updating the inode flag
>   	 * 1 for deleting the orphan
> +	 *
> +	 * Must not fail on a full qgroup either: the reservations for the
> +	 * items dropped above are only released at commit.
>   	 */
> -	trans = btrfs_start_transaction(root, 2);
> +	trans = btrfs_start_transaction_fallback_global_rsv(root, 2);
>   	if (IS_ERR(trans)) {
>   		ret = PTR_ERR(trans);
>   		trans = NULL;
> 
> base-commit: 08df884136f1c1197bab2a27814404fd329d9aac