On Wed, Jul 08, 2020 at 02:24:46PM +0800, Qu Wenruo wrote:
> -int btrfs_qgroup_reserve_data(struct btrfs_inode *inode,
> +static int try_flush_qgroup(struct btrfs_root *root)
> +{
> + struct btrfs_trans_handle *trans;
> + int ret;
> +
> + /*
> + * We don't want to run flush again and again, so if there is a running
> + * one, we won't try to start a new flush, but exit directly.
> + */
> + ret = mutex_trylock(&root->qgroup_flushing_mutex);
> + if (!ret) {
> + mutex_lock(&root->qgroup_flushing_mutex);
> + mutex_unlock(&root->qgroup_flushing_mutex);
This is abuse of mutex, for status tracking "is somebody flushing" and
for waiting until it's over.
We have many root::status bits (the BTRFS_ROOT_* namespace) so that
qgroups are flushing should another one. The bit atomically set when it
starts and cleared when it ends.
All waiting tasks should queue in a normal wait_queue_head.
> + return 0;
> + }
> +
> + ret = btrfs_start_delalloc_snapshot(root);
> + if (ret < 0)
> + goto unlock;
> + btrfs_wait_ordered_extents(root, U64_MAX, 0, (u64)-1);
> +
> + trans = btrfs_join_transaction(root);
> + if (IS_ERR(trans)) {
> + ret = PTR_ERR(trans);
> + goto unlock;
> + }
> +
> + ret = btrfs_commit_transaction(trans);
> +unlock:
> + mutex_unlock(&root->qgroup_flushing_mutex);
And also the whole wait/join/commit combo is in one huge mutex, that's
really an anti-pattern.
