On 2018年05月02日 20:49, Nikolay Borisov wrote:
>
>
> On 2.05.2018 15:29, Qu Wenruo wrote:
>>
>>
>> On 2018年05月02日 19:52, Nikolay Borisov wrote:
>>> Originally commit 2681e00f00fe ("btrfs-progs: check for matchingi
>>> free space in cache") added the account_super_bytes function to prevent
>>> false negative when running btrfs check. Turns out this function is
>>> really copied exclude_super_stripes, excluding the calls to
>>> exclude_super_stripes. Later commit e4797df6a9fa ("btrfs-progs: check
>>> the free space tree in btrfsck") introduced proper version of
>>> exclude_super_stripes. Instead of duplicating the function, just remove
>>> account_super_bytes and use exclude_super_stripes instead of the former.
>>> This also has the benefit of bringing the userspace code a bit closer
>>> to the kernel counterpart.
>>>
>>> Signed-off-by: Nikolay Borisov <nbori...@suse.com>
>>
>> Since these two functions are the same, it's completely fine to remove one.
>
> As I mentioned, they are not "comlpetely" the same. The difference is
> that exclude_super_stripes will mark the stripes as
> EXTENT_UPTODATE in fs_info->pinned_extents via add_excluded_extent.
> Dunno if this has any repercussions on functionality. I've run the progs
> test suite and didn't observe any regressions. Also looking at the usage
> of fs_info->pinned_extents didn't see anything conspicuous.
Yeah, still same conclusion here.
All pinned_extents usage I found is either really for pinned extents of
current transaction (EXTENT_DIRTY) or this excluded usage (EXTENT_UPTODATE).
And unlike EXTENT_DIRTY, EXTENT_UPTODATE won't be removed after
transaction commit, so if I didn't miss anything important, it should be OK.
Just adding Su for this, as he worked on pinning down tree blocks for
lowmem mode extent init re-init, he may be more experienced in this field.
Despite that, such abuse of EXTENT_* bits in different trees at least
need extra comment for them later.
Thanks,
Qu
>
>>
>> Reviewed-by: Qu Wenruo <w...@suse.com>
>>
>> Thanks,
>> Qu
>>
>>> ---
>>> extent-tree.c | 52 ++--------------------------------------------------
>>> 1 file changed, 2 insertions(+), 50 deletions(-)
>>>
>>> diff --git a/extent-tree.c b/extent-tree.c
>>> index ea205ccf4c30..391f0a784710 100644
>>> --- a/extent-tree.c
>>> +++ b/extent-tree.c
>>> @@ -3164,54 +3164,6 @@ static int find_first_block_group(struct btrfs_root
>>> *root,
>>> return ret;
>>> }
>>>
>>> -static void account_super_bytes(struct btrfs_fs_info *fs_info,
>>> - struct btrfs_block_group_cache *cache)
>>> -{
>>> - u64 bytenr;
>>> - u64 *logical;
>>> - int stripe_len;
>>> - int i, nr, ret;
>>> -
>>> - if (cache->key.objectid < BTRFS_SUPER_INFO_OFFSET) {
>>> - stripe_len = BTRFS_SUPER_INFO_OFFSET - cache->key.objectid;
>>> - cache->bytes_super += stripe_len;
>>> - }
>>> -
>>> - for (i = 0; i < BTRFS_SUPER_MIRROR_MAX; i++) {
>>> - bytenr = btrfs_sb_offset(i);
>>> - ret = btrfs_rmap_block(fs_info,
>>> - cache->key.objectid, bytenr,
>>> - 0, &logical, &nr, &stripe_len);
>>> - if (ret)
>>> - return;
>>> -
>>> - while (nr--) {
>>> - u64 start, len;
>>> -
>>> - if (logical[nr] > cache->key.objectid +
>>> - cache->key.offset)
>>> - continue;
>>> -
>>> - if (logical[nr] + stripe_len <= cache->key.objectid)
>>> - continue;
>>> -
>>> - start = logical[nr];
>>> - if (start < cache->key.objectid) {
>>> - start = cache->key.objectid;
>>> - len = (logical[nr] + stripe_len) - start;
>>> - } else {
>>> - len = min_t(u64, stripe_len,
>>> - cache->key.objectid +
>>> - cache->key.offset - start);
>>> - }
>>> -
>>> - cache->bytes_super += len;
>>> - }
>>> -
>>> - kfree(logical);
>>> - }
>>> -}
>>> -
>>> int btrfs_read_block_groups(struct btrfs_root *root)
>>> {
>>> struct btrfs_path *path;
>>> @@ -3287,7 +3239,7 @@ int btrfs_read_block_groups(struct btrfs_root *root)
>>> if (btrfs_chunk_readonly(info, cache->key.objectid))
>>> cache->ro = 1;
>>>
>>> - account_super_bytes(info, cache);
>>> + exclude_super_stripes(root, cache);
>>>
>>> ret = update_space_info(info, cache->flags, found_key.offset,
>>> btrfs_block_group_used(&cache->item),
>>> @@ -3331,7 +3283,7 @@ btrfs_add_block_group(struct btrfs_fs_info *fs_info,
>>> u64 bytes_used, u64 type,
>>> cache->flags = type;
>>> btrfs_set_block_group_flags(&cache->item, type);
>>>
>>> - account_super_bytes(fs_info, cache);
>>> + exclude_super_stripes(fs_info->extent_root, cache);
>>> ret = update_space_info(fs_info, cache->flags, size, bytes_used,
>>> &cache->space_info);
>>> BUG_ON(ret);
>>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
