On 2020/7/10 上午12:32, David Sterba wrote:
> 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.
In fact, I want to avoid plain wait_queue usage if possible.
Unlike mutex, wait_queue doesn't have good enough debug mechanism like
lockdep.
I see no reason re-implementing the existing mutex code by ourselves
could bring any benefit here.
It may looks like an abuse of mutex, but I could wrap it into something
like wait_or_lock_mutex(), which may slightly improve the readability.
Or am I missing anything?
>
> 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.
>
But that mutex is per-root, and is the slow path.
Converting to wait_queue won't change the pattern either.
Thanks,
Qu
Attachment:
signature.asc
Description: OpenPGP digital signature
