mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] btrfs: don't force the filesystem read-only on EDQUOT/ENOSPC verity rollback
@ 2026-09-14  6:24 Daniel Linjama
  2026-09-14  6:32 ` Qu Wenruo
  0 siblings, 1 reply; 4+ messages in thread
From: Daniel Linjama @ 2026-09-14  6:24 UTC (permalink / raw)
  To: linux-btrfs; +Cc: David Sterba, Chris Mason, linux-kernel, Daniel Linjama

When enable_verity() hits the qgroup limit, rollback_verity() needs its
own metadata reservation. When the qgroup limit refuses the rollback,
the whole filesystem is forced read-only even though the qgroup limit
was for one subvolume only.

Skip btrfs_handle_fs_error() for -EDQUOT/-ENOSPC and just return the
error. The verity orphan item is left in place but the orphan cleanup
from commit 705242538ff3 ("btrfs: verity metadata orphan items") will
remove it at the next mount. fsverity enable still correctly fails but
the filesystem is not forced read-only.

Fixes: 146054090b08 ("btrfs: initial fsverity support")
Signed-off-by: Daniel Linjama <daniel@dev.linjama.com>
---
Verified on a virtual machine with a mainline kernel (08df884136f1) and
a loop-mounted btrfs, with either a qgroup limit on a subvolume or a
full filesystem. On the unpatched kernel the whole filesystem was forced
read-only. On the patched kernel fsverity enable returns -EDQUOT/-ENOSPC
and the filesystem stays read-write.

 fs/btrfs/verity.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/verity.c b/fs/btrfs/verity.c
index 4e0ab5842274..8d0f9eea4c39 100644
--- a/fs/btrfs/verity.c
+++ b/fs/btrfs/verity.c
@@ -441,7 +441,9 @@ static int del_orphan(struct btrfs_trans_handle *trans, struct btrfs_inode *inod
  *
  * We try to handle recoverable errors while enabling verity by rolling it back
  * and just failing the operation, rather than having an fs level error no
- * matter what. However, any error in rollback is unrecoverable.
+ * matter what. Failing to delete the verity items for lack of space is
+ * tolerated, the orphan item ensures they are removed on the next mount.
+ * Any other error in rollback is unrecoverable.
  *
  * Returns 0 on success, negative error code on failure.
  */
@@ -456,9 +458,21 @@ static int rollback_verity(struct btrfs_inode *inode)
 	clear_bit(BTRFS_INODE_VERITY_IN_PROGRESS, &inode->runtime_flags);
 	ret = btrfs_drop_verity_items(inode);
 	if (ret) {
-		btrfs_handle_fs_error(root->fs_info, ret,
-				"failed to drop verity items in rollback %llu",
-				inode->vfs_inode.i_ino);
+		/*
+		 * -EDQUOT and -ENOSPC mean we could not reserve metadata to
+		 * delete the verity items. That is not a consistency problem,
+		 * so don't turn the filesystem read-only. Leave the items and
+		 * the orphan in place, orphan cleanup on the next mount will
+		 * remove them.
+		 */
+		if (ret != -EDQUOT && ret != -ENOSPC)
+			btrfs_handle_fs_error(root->fs_info, ret,
+					      "failed to drop verity items in rollback %llu",
+					      inode->vfs_inode.i_ino);
+		else
+			btrfs_warn(root->fs_info,
+				   "failed to drop verity items in rollback %llu: %pe",
+				   inode->vfs_inode.i_ino, ERR_PTR(ret));
 		goto out;
 	}
 
-- 
2.55.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] btrfs: don't force the filesystem read-only on EDQUOT/ENOSPC verity rollback
  2026-09-14  6:24 [PATCH] btrfs: don't force the filesystem read-only on EDQUOT/ENOSPC verity rollback Daniel Linjama
@ 2026-09-14  6:32 ` Qu Wenruo
  2026-09-14  9:15   ` Qu Wenruo
  0 siblings, 1 reply; 4+ messages in thread
From: Qu Wenruo @ 2026-09-14  6:32 UTC (permalink / raw)
  To: Daniel Linjama, linux-btrfs; +Cc: David Sterba, Chris Mason, linux-kernel



在 2026/9/14 15:54, Daniel Linjama 写道:
> When enable_verity() hits the qgroup limit, rollback_verity() needs its
> own metadata reservation. When the qgroup limit refuses the rollback,
> the whole filesystem is forced read-only even though the qgroup limit
> was for one subvolume only.
> 
> Skip btrfs_handle_fs_error() for -EDQUOT/-ENOSPC and just return the
> error. The verity orphan item is left in place but the orphan cleanup
> from commit 705242538ff3 ("btrfs: verity metadata orphan items") will
> remove it at the next mount. fsverity enable still correctly fails but
> the filesystem is not forced read-only.
> 
> Fixes: 146054090b08 ("btrfs: initial fsverity support")
> Signed-off-by: Daniel Linjama <daniel@dev.linjama.com>
> ---
> Verified on a virtual machine with a mainline kernel (08df884136f1) and
> a loop-mounted btrfs, with either a qgroup limit on a subvolume or a
> full filesystem. On the unpatched kernel the whole filesystem was forced
> read-only. On the patched kernel fsverity enable returns -EDQUOT/-ENOSPC
> and the filesystem stays read-write.

Please also submit a fstests case for it.

> 
>   fs/btrfs/verity.c | 22 ++++++++++++++++++----
>   1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/verity.c b/fs/btrfs/verity.c
> index 4e0ab5842274..8d0f9eea4c39 100644
> --- a/fs/btrfs/verity.c
> +++ b/fs/btrfs/verity.c
> @@ -441,7 +441,9 @@ static int del_orphan(struct btrfs_trans_handle *trans, struct btrfs_inode *inod
>    *
>    * We try to handle recoverable errors while enabling verity by rolling it back
>    * and just failing the operation, rather than having an fs level error no
> - * matter what. However, any error in rollback is unrecoverable.
> + * matter what. Failing to delete the verity items for lack of space is
> + * tolerated, the orphan item ensures they are removed on the next mount.
> + * Any other error in rollback is unrecoverable.
>    *
>    * Returns 0 on success, negative error code on failure.
>    */
> @@ -456,9 +458,21 @@ static int rollback_verity(struct btrfs_inode *inode)
>   	clear_bit(BTRFS_INODE_VERITY_IN_PROGRESS, &inode->runtime_flags);
>   	ret = btrfs_drop_verity_items(inode);
>   	if (ret) {
> -		btrfs_handle_fs_error(root->fs_info, ret,
> -				"failed to drop verity items in rollback %llu",
> -				inode->vfs_inode.i_ino);
> +		/*
> +		 * -EDQUOT and -ENOSPC mean we could not reserve metadata to
> +		 * delete the verity items. That is not a consistency problem,
> +		 * so don't turn the filesystem read-only. Leave the items and
> +		 * the orphan in place, orphan cleanup on the next mount will
> +		 * remove them.
> +		 */

EDQUOT is fine, but I'm not sure if ENOSPC is really recoverable.

Even if we ignore ENOSPC for now, we may have really exhausted the 
metadata, and the next transaction may be aborted anyway.

I believe your test case is only utilizing qgroup limit, or did you hit 
a case where btrfs_drop_verity_items() returns -ENOSPC and the fs still 
works fine?

Otherwise the idea looks good to me.

Thanks,
Qu
> +		if (ret != -EDQUOT && ret != -ENOSPC)
> +			btrfs_handle_fs_error(root->fs_info, ret,
> +					      "failed to drop verity items in rollback %llu",
> +					      inode->vfs_inode.i_ino);
> +		else
> +			btrfs_warn(root->fs_info,
> +				   "failed to drop verity items in rollback %llu: %pe",
> +				   inode->vfs_inode.i_ino, ERR_PTR(ret));
>   		goto out;
>   	}
>   


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] btrfs: don't force the filesystem read-only on EDQUOT/ENOSPC verity rollback
  2026-09-14  6:32 ` Qu Wenruo
@ 2026-09-14  9:15   ` Qu Wenruo
  2026-09-15  6:07     ` Daniel Linjama
  0 siblings, 1 reply; 4+ messages in thread
From: Qu Wenruo @ 2026-09-14  9:15 UTC (permalink / raw)
  To: Daniel Linjama, linux-btrfs; +Cc: David Sterba, Chris Mason, linux-kernel



在 2026/9/14 16:02, Qu Wenruo 写道:
> 
> 
> 在 2026/9/14 15:54, Daniel Linjama 写道:
>> When enable_verity() hits the qgroup limit, rollback_verity() needs its
>> own metadata reservation. When the qgroup limit refuses the rollback,
>> the whole filesystem is forced read-only even though the qgroup limit
>> was for one subvolume only.
>>
>> Skip btrfs_handle_fs_error() for -EDQUOT/-ENOSPC and just return the
>> error. The verity orphan item is left in place but the orphan cleanup
>> from commit 705242538ff3 ("btrfs: verity metadata orphan items") will
>> remove it at the next mount. fsverity enable still correctly fails but
>> the filesystem is not forced read-only.
>>
>> Fixes: 146054090b08 ("btrfs: initial fsverity support")
>> Signed-off-by: Daniel Linjama <daniel@dev.linjama.com>
>> ---
>> Verified on a virtual machine with a mainline kernel (08df884136f1) and
>> a loop-mounted btrfs, with either a qgroup limit on a subvolume or a
>> full filesystem. On the unpatched kernel the whole filesystem was forced
>> read-only. On the patched kernel fsverity enable returns -EDQUOT/-ENOSPC
>> and the filesystem stays read-write.
> 
> Please also submit a fstests case for it.
> 
>>
>>   fs/btrfs/verity.c | 22 ++++++++++++++++++----
>>   1 file changed, 18 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/btrfs/verity.c b/fs/btrfs/verity.c
>> index 4e0ab5842274..8d0f9eea4c39 100644
>> --- a/fs/btrfs/verity.c
>> +++ b/fs/btrfs/verity.c
>> @@ -441,7 +441,9 @@ static int del_orphan(struct btrfs_trans_handle 
>> *trans, struct btrfs_inode *inod
>>    *
>>    * We try to handle recoverable errors while enabling verity by 
>> rolling it back
>>    * and just failing the operation, rather than having an fs level 
>> error no
>> - * matter what. However, any error in rollback is unrecoverable.
>> + * matter what. Failing to delete the verity items for lack of space is
>> + * tolerated, the orphan item ensures they are removed on the next 
>> mount.
>> + * Any other error in rollback is unrecoverable.
>>    *
>>    * Returns 0 on success, negative error code on failure.
>>    */
>> @@ -456,9 +458,21 @@ static int rollback_verity(struct btrfs_inode 
>> *inode)
>>       clear_bit(BTRFS_INODE_VERITY_IN_PROGRESS, &inode->runtime_flags);
>>       ret = btrfs_drop_verity_items(inode);
>>       if (ret) {
>> -        btrfs_handle_fs_error(root->fs_info, ret,
>> -                "failed to drop verity items in rollback %llu",
>> -                inode->vfs_inode.i_ino);
>> +        /*
>> +         * -EDQUOT and -ENOSPC mean we could not reserve metadata to
>> +         * delete the verity items. That is not a consistency problem,
>> +         * so don't turn the filesystem read-only. Leave the items and
>> +         * the orphan in place, orphan cleanup on the next mount will
>> +         * remove them.
>> +         */
> 
> EDQUOT is fine, but I'm not sure if ENOSPC is really recoverable.
> 
> Even if we ignore ENOSPC for now, we may have really exhausted the 
> metadata, and the next transaction may be aborted anyway.

Sashiko also mentioned even if we ignored EDQUOT for now, the next mount 
we still fallback to call btrfs_drop_verity_items() on the inode cleanup.

And it will return -EDQUOT again, so the inode will not be cleanred up.

Considering we're only delete items, I think the better solution would 
be calling btrfs_start_transaction_fallback_global_rsv(), which will not 
enforce qgroup, thus should not return -EDQUOT in the first place.

> 
> I believe your test case is only utilizing qgroup limit, or did you hit 
> a case where btrfs_drop_verity_items() returns -ENOSPC and the fs still 
> works fine?
> 
> Otherwise the idea looks good to me.
> 
> Thanks,
> Qu
>> +        if (ret != -EDQUOT && ret != -ENOSPC)
>> +            btrfs_handle_fs_error(root->fs_info, ret,
>> +                          "failed to drop verity items in rollback 
>> %llu",
>> +                          inode->vfs_inode.i_ino);
>> +        else
>> +            btrfs_warn(root->fs_info,
>> +                   "failed to drop verity items in rollback %llu: %pe",
>> +                   inode->vfs_inode.i_ino, ERR_PTR(ret));
>>           goto out;
>>       }
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] btrfs: don't force the filesystem read-only on EDQUOT/ENOSPC verity rollback
  2026-09-14  9:15   ` Qu Wenruo
@ 2026-09-15  6:07     ` Daniel Linjama
  0 siblings, 0 replies; 4+ messages in thread
From: Daniel Linjama @ 2026-09-15  6:07 UTC (permalink / raw)
  To: Qu Wenruo
  Cc: linux-btrfs, David Sterba, Chris Mason, linux-kernel, Daniel Linjama

On 2026/9/14 18:45, Qu Wenruo wrote:
> On 2026/9/14 16:02, Qu Wenruo wrote:
>> Please also submit a fstests case for it.

Sent to fstests as btrfs/354 (qgroup limit) and btrfs/355 (full
filesystem):
https://lore.kernel.org/fstests/20260915053815.307674-1-daniel@dev.linjama.com/

>> EDQUOT is fine, but I'm not sure if ENOSPC is really recoverable.
>>
>> Even if we ignore ENOSPC for now, we may have really exhausted the
>> metadata, and the next transaction may be aborted anyway.
>
> Sashiko also mentioned even if we ignored EDQUOT for now, the next mount
> we still fallback to call btrfs_drop_verity_items() on the inode cleanup.
>
> And it will return -EDQUOT again, so the inode will not be cleanred up.
>
> Considering we're only delete items, I think the better solution would
> be calling btrfs_start_transaction_fallback_global_rsv(), which will not
> enforce qgroup, thus should not return -EDQUOT in the first place.

You are both right. With v1 the next mount fails the orphan cleanup with
-EDQUOT again, every lookup of the subvolume fails, and mounting it with
-o subvol fails too, so the subvolume is unreachable as long as the limit
is in place.

v2 does what you suggest: btrfs_start_transaction_fallback_global_rsv()
in drop_verity_items(), for the second transaction in rollback_verity()
and for the orphan item deletion in btrfs_orphan_cleanup(). All three are
needed, the reservations of the dropped items are only released at
commit, so the following transactions hit the limit as well.
https://lore.kernel.org/linux-btrfs/20260915055109.311839-1-daniel@dev.linjama.com/

>> I believe your test case is only utilizing qgroup limit, or did you hit
>> a case where btrfs_drop_verity_items() returns -ENOSPC and the fs still
>> works fine?

Yes, on a full mixed block group filesystem the rollback fails with
-ENOSPC the same way. Without the fix the orphan cleanup then fails the
next read-write mount (open_ctree failed: -ENOSPC), and the filesystem
can only be mounted read-only after that. With v2 the mount succeeds and
the leftover items are dropped. btrfs/355 covers this.

Thanks,
Daniel

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-15  6:08 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-14  6:24 [PATCH] btrfs: don't force the filesystem read-only on EDQUOT/ENOSPC verity rollback Daniel Linjama
2026-09-14  6:32 ` Qu Wenruo
2026-09-14  9:15   ` Qu Wenruo
2026-09-15  6:07     ` Daniel Linjama

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®