Re: [PATCH 1/3] btrfs: Introduce extent_changeset_revert() for qgroup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 2020/7/2 下午9:40, Josef Bacik wrote:
> On 7/1/20 8:14 PM, Qu Wenruo wrote:
>> [PROBLEM]
>> Before this patch, when btrfs_qgroup_reserve_data() fails, we free all
>> reserved space of the changeset.
>>
>> This means the following call is not possible:
>>     ret = btrfs_qgroup_reserve_data();
>>     if (ret == -EDQUOT) {
>>         /* Do something to free some qgroup space */
>>         ret = btrfs_qgroup_reserve_data();
>>     }
>>
>> As if the first btrfs_qgroup_reserve_data() fails, it will free all
>> reserved qgroup space, so the next btrfs_qgroup_reserve_data() will
>> always success, and can go beyond qgroup limit.
>>
>> [CAUSE]
>> This is mostly due to the fact that we don't have a good way to revert
>> changeset modification accurately.
>>
>> Currently the changeset infrastructure is implemented using ulist, which
>> can only store two u64 values, used as start and length for each changed
>> extent range.
>>
>> So we can't record which changed extent is modified in last
>> modification, thus unable to revert to previous status.
>>
>> [FIX]
>> This patch will re-implement using pure rbtree, adding a new member,
>> changed_extent::seq, so we can remove changed extents which is
>> modified in previous modification.
>>
>> This allows us to implement qgroup_revert(), which allow btrfs to revert
>> its modification to the io_tree.
>>
> 
> I'm having a hard time groking what's going on here.  These changesets
> are limited to a [start, end] range correct?

Yes, but we may be only responsible for part of the changeset.

One example is we want to falloc range [0, 16K)
And [0, 4K), [8K, 12K) has already one existing file extent.

Then we succeeded in allocating space for [4K, 8K), but failed to
allocating space for [8K, 12K).

In that case, if we just return EDQUOT and clear the range for [4K, 8k)
and [8K, 12K), everything is completely fine.

But if we want to retry, then we should only clear the range for [8K,
12K), but not to clear [4K, 8K).

That's what we need for the next patch, just revert what we did in
previous set_extent_bit(), but not touching previously set bits.

Thanks,
Qu

>  Why can we have multiple
> changesets for the same range?  Are we reserving doubly for
> modifications in the same range?  Because it seems here if you find your
> changeset at all we'll clear the io_tree, but if you can have multiple
> changesets for the same range then....why even bother?  Thanks,
> 
> Josef

Attachment: signature.asc
Description: OpenPGP digital signature


[Index of Archives]     [Linux Filesystem Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux