btrfs: fix squota accounting during enable generation

The first transaction that enables squotas is special and a bit tricky.
We have to set BTRFS_FS_QUOTA_ENABLED after the transaction to avoid a
deadlock, so any delayed refs that run before we set the bit are not
squota accounted. For data this is fine, we don't get an owner_ref, so
there is no real harm, it's as if the extent predated squotas. However
for metadata, the tree block will have gen == enable_gen so when we free
it later, we will decrement the squota accounting, which can result in
an underflow. Before it is freed, btrfs check shows errors, as we have
mismatched usage between the node generations/owners and the squota
values.

There are two angles to this fix:

1. For extents that come in delayed_refs that run during the
   enable_gen transaction, we must actually set enable_gen to the *next*
   transaction. That is the first transaction that we can really
   properly account in any way.
2. For extents that come in between the end of our transaction handle
   and the time we set the BTRFS_FS_QUOTA_ENABLED bit, we need an
   additional bit, BTRFS_FS_SQUOTA_ENABLING which only affects recording
   squota deltas, so we do pick up those extents. Otherwise, we would
   miss them, even for enable_gen + 1.

Fixes: bd7c1ea3a302 ("btrfs: qgroup: check generation when recording simple quota delta")
Reviewed-by: Qu Wenruo <wqu@suse.com>
Signed-off-by: Boris Burkov <boris@bur.io>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>

diff --git a/fs/btrfs/fs.h b/fs/btrfs/fs.h
index a4758d94b32e948b30476785366e67a1abc2335b..a8aa086a4df865818e1adfeff7cff604681ec3b1 100644 (file)
--- a/fs/btrfs/fs.h
+++ b/fs/btrfs/fs.h
@@ -155,6 +155,7 @@ enum {
        BTRFS_FS_LOG_RECOVERING,
        BTRFS_FS_OPEN,
        BTRFS_FS_QUOTA_ENABLED,
+       BTRFS_FS_SQUOTA_ENABLING,
        BTRFS_FS_UPDATE_UUID_TREE_GEN,
        BTRFS_FS_CREATING_FREE_SPACE_TREE,
        BTRFS_FS_BTREE_ERR,

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 86c036a089f6d90300a6b1208c5237752f93e779..5f33727a79722123dad7e2136ffc0d25734b9abb 100644 (file)
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -1107,7 +1107,13 @@ int btrfs_quota_enable(struct btrfs_fs_info *fs_info,
        if (simple) {
                fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_SIMPLE_MODE;
                btrfs_set_fs_incompat(fs_info, SIMPLE_QUOTA);
-               btrfs_set_qgroup_status_enable_gen(leaf, ptr, trans->transid);
+               /*
+                * Set the enable generation to the next transaction, as we cannot
+                * ensure that extents written during this transaction will see any
+                * state we have set here. So we should treat all extents of the
+                * transaction as coming in before squotas was enabled.
+                */
+               btrfs_set_qgroup_status_enable_gen(leaf, ptr, trans->transid + 1);
        } else {
                fs_info->qgroup_flags |= BTRFS_QGROUP_STATUS_FLAG_INCONSISTENT;
        }
@@ -1210,7 +1216,15 @@ out_add_root:
                goto out_free_path;
        }
 
-       fs_info->qgroup_enable_gen = trans->transid;
+       /*
+        * Set fs_info->qgroup_enable_gen and BTRFS_FS_SQUOTA_ENABLING
+        * under the transaction handle. We want to ensure that all extents in
+        * the next transaction definitely see them.
+        */
+       if (simple) {
+               fs_info->qgroup_enable_gen = trans->transid + 1;
+               set_bit(BTRFS_FS_SQUOTA_ENABLING, &fs_info->flags);
+       }
 
        mutex_unlock(&fs_info->qgroup_ioctl_lock);
        /*
@@ -1224,9 +1238,15 @@ out_add_root:
         */
        ret = btrfs_commit_transaction(trans);
        trans = NULL;
+
        mutex_lock(&fs_info->qgroup_ioctl_lock);
-       if (ret)
+       if (ret) {
+               if (simple) {
+                       clear_bit(BTRFS_FS_SQUOTA_ENABLING, &fs_info->flags);
+                       fs_info->qgroup_enable_gen = 0;
+               }
                goto out_free_path;
+       }
 
        /*
         * Set quota enabled flag after committing the transaction, to avoid
@@ -1236,6 +1256,8 @@ out_add_root:
        spin_lock(&fs_info->qgroup_lock);
        fs_info->quota_root = quota_root;
        set_bit(BTRFS_FS_QUOTA_ENABLED, &fs_info->flags);
+       if (simple)
+               clear_bit(BTRFS_FS_SQUOTA_ENABLING, &fs_info->flags);
        spin_unlock(&fs_info->qgroup_lock);
 
        /* Skip rescan for simple qgroups. */
@@ -4922,7 +4944,8 @@ int btrfs_record_squota_delta(struct btrfs_fs_info *fs_info,
        u64 num_bytes = delta->num_bytes;
        const int sign = (delta->is_inc ? 1 : -1);
 
-       if (btrfs_qgroup_mode(fs_info) != BTRFS_QGROUP_MODE_SIMPLE)
+       if (btrfs_qgroup_mode(fs_info) != BTRFS_QGROUP_MODE_SIMPLE &&
+           !test_bit(BTRFS_FS_SQUOTA_ENABLING, &fs_info->flags))
                return 0;
 
        if (!btrfs_is_fstree(root))