Re: [PATCH] btrfs: relocation: output warning message for leftover v1 space cache before aborting current data balance

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

On 2020/12/29 下午9:17, Stéphane Lesimple wrote:
> December 29, 2020 2:08 PM, "Qu Wenruo" <quwenruo.btrfs@gmx.com> wrote:
>
>> On 2020/12/29 下午8:51, Stéphane Lesimple wrote:
>>
>>> December 29, 2020 1:43 PM, "Qu Wenruo" <wqu@suse.com> wrote:
>>>
>>>> On 2020/12/29 下午8:30, Stéphane Lesimple wrote:
>>>
>>> December 29, 2020 12:32 PM, "Qu Wenruo" <quwenruo.btrfs@gmx.com> wrote:
>>>
>>> On 2020/12/29 下午7:08, Stéphane Lesimple wrote:
>>>
>>> December 29, 2020 11:31 AM, "Qu Wenruo" <quwenruo.btrfs@gmx.com> wrote:
>>>
>>> # btrfs ins dump-tree -t root /dev/mapper/luks-tank-mdata | grep EXTENT_DA
>>> item 27 key (51933 EXTENT_DATA 0) itemoff 9854 itemsize 53
>>> item 12 key (72271 EXTENT_DATA 0) itemoff 14310 itemsize 53
>>> item 25 key (74907 EXTENT_DATA 0) itemoff 12230 itemsize 53
>>> Mind to dump all those related inodes?
>>>
>>> E.g:
>>>
>>> $ btrfs ins dump-tree -t root <dev> | gerp 51933 -C 10
>>>
>>> Sure. I added -w to avoid grepping big numbers just containing the digits 51933:
>>>
>>> # btrfs ins dump-tree -t root /dev/mapper/luks-tank-mdata | grep 51933 -C 10 -w
>>> generation 2614632 root_dirid 256 bytenr 42705449811968 level 2 refs 1
>>> lastsnap 2614456 byte_limit 0 bytes_used 101154816 flags 0x1(RDONLY)
>>> uuid 1100ff6c-45fa-824d-ad93-869c94a87c7b
>>> parent_uuid 8bb8a884-ea4f-d743-8b0c-b6fdecbc397c
>>> ctransid 1337630 otransid 1249372 stransid 0 rtransid 0
>>> ctime 1554266422.693480985 (2019-04-03 06:40:22)
>>> otime 1547877605.465117667 (2019-01-19 07:00:05)
>>> drop key (0 UNKNOWN.0 0) level 0
>>> item 25 key (51098 ROOT_BACKREF 5) itemoff 10067 itemsize 42
>>> root backref key dirid 534 sequence 22219 name 20190119_070006_hourly.7
>>> item 26 key (51933 INODE_ITEM 0) itemoff 9907 itemsize 160
>>> generation 0 transid 1517381 size 262144 nbytes 30553407488
>>> block group 0 mode 100600 links 1 uid 0 gid 0 rdev 0
>>> sequence 116552 flags 0x1b(NODATASUM|NODATACOW|NOCOMPRESS|PREALLOC)
>>> atime 0.0 (1970-01-01 01:00:00)
>>> ctime 1567904822.739884119 (2019-09-08 03:07:02)
>>> mtime 0.0 (1970-01-01 01:00:00)
>>> otime 0.0 (1970-01-01 01:00:00)
>>> item 27 key (51933 EXTENT_DATA 0) itemoff 9854 itemsize 53
>>> generation 1517381 type 2 (prealloc)
>>> prealloc data disk byte 34626327621632 nr 262144
>>>> Got the point now.
>>>>
>>>> The type is preallocated, which means we haven't yet written space cache
>>>> into it.
>>>>
>>>> But the code only checks the regular file extent (written, not
>>>> preallocated).
>>>>
>>>> So the proper fix would looks like this:
>>>>
>>>> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
>>>> index 19b7db8b2117..1d73d7c2fbd7 100644
>>>> --- a/fs/btrfs/relocation.c
>>>> +++ b/fs/btrfs/relocation.c
>>>> @@ -2975,11 +2975,14 @@ static int delete_v1_space_cache(struct
>>>> extent_buffer *leaf,
>>>> return 0;
>>>>
>>>> for (i = 0; i < btrfs_header_nritems(leaf); i++) {
>>>> + u8 type;
>>>> btrfs_item_key_to_cpu(leaf, &key, i);
>>>> if (key.type != BTRFS_EXTENT_DATA_KEY)
>>>> continue;
>>>> ei = btrfs_item_ptr(leaf, i, struct
>>>> btrfs_file_extent_item);
>>>> - if (btrfs_file_extent_type(leaf, ei) ==
>>>> BTRFS_FILE_EXTENT_REG &&
>>>> + type = btrfs_file_extent_type(leaf, ei);
>>>> + if ((type == BTRFS_FILE_EXTENT_REG ||
>>>> + type == BTRFS_FILE_EXTENT_PREALLOC) &&
>>>> btrfs_file_extent_disk_bytenr(leaf, ei) ==
>>>> data_bytenr) {
>>>> found = true;
>>>> space_cache_ino = key.objectid;
>>>>
>>>> With this, the relocation should finish without problem.
>>>
>>> Aaah, it makes sense indeed.
>>>
>>> Do you want me to try this fix right now, or do you want to have a look
>>> at the btrfs-progs crash first? I don't know if it's related, but if it is,
>>> then maybe I won't be able to reproduce it again after finishing the balance.
>>
>> The problem is, I'm not that confident with v2 space cache debugging,
>> thus can't help much.
>>
>> But at least, the problem you're reporting doesn't really need a
>> btrfs-check repair.
>> Just a kernel fix would be enough.
>
> Yes indeed.
>
>>> I don't mind leaving the FS in this state for a few more days/weeks if needed.
>>
>> That's fine if you want some one to look into the btrfs-progs bug.
>
> I'll leave it like this up to, let's say, mid-January.

BTW, would you mind to dump the root tree?

# btrfs ins dump-tree -t root <device>

I'm more interested in the half dropped v1 space cache. Currently
although they are sane (with proper inode items), I'm a little concerned
if current v1 space cache cleanup can remove them.

Feel free to remove any subvolume names.

Thanks,
Qu

> If nobody has expressed interest
> in looking into this other problem, I'll apply your final patch and complete the balance,
> so that you can add a Tested-By.
>
>> I'll submit a proper bug fix soon.
>
> Thanks,
>
> Good evening!
>