From: Filipe Manana <fdmanana@gmail.com>
To: Qu Wenruo <quwenruo.btrfs@gmx.com>
Cc: Qu Wenruo <wqu@suse.com>,
linux-btrfs <linux-btrfs@vger.kernel.org>,
Jiachen YANG <farseerfc@gmail.com>
Subject: Re: [PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size
Date: Wed, 17 Feb 2021 12:47:40 +0000 [thread overview]
Message-ID: <CAL3q7H6keR5t67W0MEkrYjPA_uQJBjrNjfxgn+vfdZDrhi-gJw@mail.gmail.com> (raw)
In-Reply-To: <581edba6-9435-1488-630e-7c2ab1ee7b83@gmx.com>
On Wed, Feb 17, 2021 at 12:29 PM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
>
>
>
> On 2021/2/17 下午8:12, Filipe Manana wrote:
> > On Wed, Feb 17, 2021 at 11:28 AM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
> >>
> >>
> >>
> >> On 2021/2/17 下午6:59, Filipe Manana wrote:
> >>> On Tue, Feb 16, 2021 at 11:42 PM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On 2021/2/16 下午10:45, Filipe Manana wrote:
> >>>>> On Wed, Jun 24, 2020 at 10:00 PM Qu Wenruo <wqu@suse.com> wrote:
> >>>>>>
> >>>>>> [BUG]
> >>>>>> The following script could lead to corrupted btrfs fs after
> >>>>>> btrfs-convert:
> >>>>>>
> >>>>>> fallocate -l 1G test.img
> >>>>>> mkfs.ext4 test.img
> >>>>>> mount test.img $mnt
> >>>>>> fallocate -l 200m $mnt/file1
> >>>>>> fallocate -l 200m $mnt/file2
> >>>>>> fallocate -l 200m $mnt/file3
> >>>>>> fallocate -l 200m $mnt/file4
> >>>>>> fallocate -l 205m $mnt/file1
> >>>>>> fallocate -l 205m $mnt/file2
> >>>>>> fallocate -l 205m $mnt/file3
> >>>>>> fallocate -l 205m $mnt/file4
> >>>>>> umount $mnt
> >>>>>> btrfs-convert test.img
> >>>>>>
> >>>>>> The result btrfs will have a device extent beyond its boundary:
> >>>>>> pening filesystem to check...
> >>>>>> Checking filesystem on test.img
> >>>>>> UUID: bbcd7399-fd5b-41a7-81ae-d48bc6935e43
> >>>>>> [1/7] checking root items
> >>>>>> [2/7] checking extents
> >>>>>> ERROR: dev extent devid 1 physical offset 993198080 len 85786624 is beyond device boundary 1073741824
> >>>>>> ERROR: errors found in extent allocation tree or chunk allocation
> >>>>>> [3/7] checking free space cache
> >>>>>> [4/7] checking fs roots
> >>>>>> [5/7] checking only csums items (without verifying data)
> >>>>>> [6/7] checking root refs
> >>>>>> [7/7] checking quota groups skipped (not enabled on this FS)
> >>>>>> found 913960960 bytes used, error(s) found
> >>>>>> total csum bytes: 891500
> >>>>>> total tree bytes: 1064960
> >>>>>> total fs tree bytes: 49152
> >>>>>> total extent tree bytes: 16384
> >>>>>> btree space waste bytes: 144885
> >>>>>> file data blocks allocated: 2129063936
> >>>>>> referenced 1772728320
> >>>>>>
> >>>>>> [CAUSE]
> >>>>>> Btrfs-convert first collect all used blocks in the original fs, then
> >>>>>> slightly enlarge the used blocks range as new btrfs data chunks.
> >>>>>>
> >>>>>> However the enlarge part has a problem, that it doesn't take the device
> >>>>>> boundary into consideration.
> >>>>>>
> >>>>>> Thus it caused device extents and data chunks to go beyond device
> >>>>>> boundary.
> >>>>>>
> >>>>>> [FIX]
> >>>>>> Just to extra check before inserting data chunks into
> >>>>>> btrfs_convert_context::data_chunk.
> >>>>>>
> >>>>>> Reported-by: Jiachen YANG <farseerfc@gmail.com>
> >>>>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
> >>>>>
> >>>>> So, having upgraded a test box from btrfs-progs v5.6.1 to v5.10.1, I
> >>>>> now have btrfs/136 (fstests) failing:
> >>>>>
> >>>>> $ ./check btrfs/136
> >>>>> FSTYP -- btrfs
> >>>>> PLATFORM -- Linux/x86_64 debian8 5.11.0-rc7-btrfs-next-81 #1 SMP
> >>>>> PREEMPT Tue Feb 16 12:29:07 WET 2021
> >>>>> MKFS_OPTIONS -- /dev/sdc
> >>>>> MOUNT_OPTIONS -- /dev/sdc /home/fdmanana/btrfs-tests/scratch_1
> >>>>>
> >>>>> btrfs/136 7s ... [failed, exit status 1]- output mismatch (see
> >>>>> /home/fdmanana/git/hub/xfstests/results//btrfs/136.out.bad)
> >>>>> --- tests/btrfs/136.out 2020-06-10 19:29:03.818519162 +0100
> >>>>> +++ /home/fdmanana/git/hub/xfstests/results//btrfs/136.out.bad
> >>>>> 2021-02-16 14:31:30.669559188 +0000
> >>>>> @@ -1,2 +1,3 @@
> >>>>> QA output created by 136
> >>>>> -Silence is golden
> >>>>> +btrfs-convert failed
> >>>>> +(see /home/fdmanana/git/hub/xfstests/results//btrfs/136.full for details)
> >>>>> ...
> >>>>> (Run 'diff -u /home/fdmanana/git/hub/xfstests/tests/btrfs/136.out
> >>>>> /home/fdmanana/git/hub/xfstests/results//btrfs/136.out.bad' to see
> >>>>> the entire diff)
> >>>>> Ran: btrfs/136
> >>>>> Failures: btrfs/136
> >>>>> Failed 1 of 1 tests
> >>>>>
> >>>>> A bisect pointed to this patch.
> >>>>> Did you get this failure on your test box as well?
> >>>>
> >>>> Nope.
> >>>>
> >>>> Just tested with btrfs-progs v5.10.1, it passes:
> >>>> $ which btrfs
> >>>> /usr/bin/btrfs
> >>>> $ btrfs --version
> >>>> btrfs-progs v5.10.1
> >>>> $ sudo ./check btrfs/136
> >>>> FSTYP -- btrfs
> >>>> PLATFORM -- Linux/x86_64 btrfs-desktop-vm 5.11.0-rc4-custom+ #4
> >>>> SMP PREEMPT Mon Jan 25 18:35:22 CST 2021
> >>>> MKFS_OPTIONS -- /dev/mapper/test-scratch1
> >>>> MOUNT_OPTIONS -- /dev/mapper/test-scratch1 /mnt/scratch
> >>>>
> >>>> btrfs/136 6s ... 10s
> >>>> Ran: btrfs/136
> >>>> Passed all 1 tests
> >>>>
> >>>> Would you mind to provide the 136.full to help debugging the failure?
> >>>
> >>> Sure, here it is (also at https://pastebin.com/XhEX2dju):
> >>>
> >>> root 10:06:39 /home/fdmanana/git/hub/xfstests (master)> cat
> >>> /home/fdmanana/git/hub/xfstests/results//btrfs/136.full
> >>> mke2fs 1.45.6 (20-Mar-2020)
> >>> Discarding device blocks: done
> >>> Creating filesystem with 5242880 4k blocks and 1310720 inodes
> >>> Filesystem UUID: 9519afc8-8ea0-42da-8ac5-3b4c20469dd1
> >>> Superblock backups stored on blocks:
> >>> 32768, 98304, 163840, 229376, 294912, 819200, 884736, 1605632, 2654208,
> >>> 4096000
> >>>
> >>> Allocating group tables: done
> >>> Writing inode tables: done
> >>> Creating journal (32768 blocks): done
> >>> Writing superblocks and filesystem accounting information: done
> >>>
> >>> Run fsstress -p 20 -n 100 -d
> >>> /home/fdmanana/btrfs-tests/scratch_1/ext3_ext4_data/ext3
> >>> tune2fs 1.45.6 (20-Mar-2020)
> >>> e2fsck 1.45.6 (20-Mar-2020)
> >>> Pass 1: Checking inodes, blocks, and sizes
> >>> Pass 2: Checking directory structure
> >>> Pass 3: Checking directory connectivity
> >>> Pass 3A: Optimizing directories
> >>> Pass 4: Checking reference counts
> >>> Pass 5: Checking group summary information
> >>>
> >>> /dev/sdc: ***** FILE SYSTEM WAS MODIFIED *****
> >>> /dev/sdc: 235/1310720 files (22.1% non-contiguous), 129757/5242880 blocks
> >>> Run fsstress -p 20 -n 100 -d
> >>> /home/fdmanana/btrfs-tests/scratch_1/ext3_ext4_data/ext4
> >>> ERROR: missing data block for bytenr 1048576
> >>> ERROR: failed to create ext2_saved/image: -2
> >>> WARNING: an error occurred during conversion, filesystem is partially
> >>
> >> The bytenr is 1M, which looks related to the super block relocation code.
> >>
> >> Thus it maybe disk layout related.
> >>
> >> Your disk used here is 20G, although I tried the same size disk, it
> >> still passes.
> >>
> >> If you could reproduce it reliably (as you can do the bisect), mind to
> >> take a e2image -Q dump?
> >
> > Yep, I can reproduce it reliably, always fails with btrfs-progs v5.7+.
> >
> > Image attached. I took it after the test converts the ext3 fs to ext4
> > and right before the test calls btrfs-convert.
>
> Something doesn't go correct.
>
> v5.10.1 btrfs-convert still passes on the image in my test env.
>
> $ ./btrfs-convert /dev/vdc
> create btrfs filesystem:
> blocksize: 4096
> nodesize: 16384
> features: extref, skinny-metadata (default)
> checksum: crc32c
> free space report:
> total: 21474836480
> free: 15690694656 (73.07%)
> creating ext2 image file
> creating btrfs metadata
> copy inodes [o] [ 4/ 527]
> conversion complete
> $ ./btrfs --version
> btrfs-progs v5.10.1
>
>
> I have no idea what's going on now...
>
> I'm afraid you have to dig the bug by yourself, as I really can't
> reproduce the bug, even with the image dump...
I'll add it to my list of things to check.
Thanks for looking into it!
>
> Thanks,
> Qu
>
> >
> > Thanks.
> >
> >>
> >> Thanks,
> >> Qu
> >>
> >>> created but not finalized and not mountable
> >>> create btrfs filesystem:
> >>> blocksize: 4096
> >>> nodesize: 16384
> >>> features: extref, skinny-metadata (default)
> >>> checksum: crc32c
> >>> creating ext2 image file
> >>> btrfs-convert failed
> >>> root 10:14:18 /home/fdmanana/git/hub/xfstests (master)>
> >>>
> >>>
> >>> Thanks
> >>>
> >>>
> >>>>
> >>>> Thanks,
> >>>> Qu
> >>>>>
> >>>>> Thanks.
> >>>>>
> >>>>>> ---
> >>>>>> convert/main.c | 2 ++
> >>>>>> 1 file changed, 2 insertions(+)
> >>>>>>
> >>>>>> diff --git a/convert/main.c b/convert/main.c
> >>>>>> index c86ddd988c63..7709e9a6c085 100644
> >>>>>> --- a/convert/main.c
> >>>>>> +++ b/convert/main.c
> >>>>>> @@ -669,6 +669,8 @@ static int calculate_available_space(struct btrfs_convert_context *cctx)
> >>>>>> cur_off = cache->start;
> >>>>>> cur_len = max(cache->start + cache->size - cur_off,
> >>>>>> min_stripe_size);
> >>>>>> + /* data chunks should never exceed device boundary */
> >>>>>> + cur_len = min(cctx->total_bytes - cur_off, cur_len);
> >>>>>> ret = add_merge_cache_extent(data_chunks, cur_off, cur_len);
> >>>>>> if (ret < 0)
> >>>>>> goto out;
> >>>>>> --
> >>>>>> 2.27.0
> >>>>>>
> >>>>>
> >>>>>
> >>>
> >>>
> >>>
> >
> >
> >
--
Filipe David Manana,
“Whether you think you can, or you think you can't — you're right.”
prev parent reply other threads:[~2021-02-17 12:48 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-24 11:55 [PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size Qu Wenruo
2020-06-24 11:55 ` [PATCH 2/2] btrfs-progs: tests/convert: Add test case to make sure we won't allocate dev extents beyond device boundary Qu Wenruo
2020-06-24 15:49 ` David Sterba
2020-06-24 15:51 ` David Sterba
2020-06-24 15:58 ` [PATCH 1/2] btrfs-progs: convert: Ensure the data chunks size never exceed device size David Sterba
2020-08-15 13:38 ` Anand Jain
2020-08-15 23:40 ` Qu Wenruo
2021-02-16 14:45 ` Filipe Manana
2021-02-16 23:42 ` Qu Wenruo
2021-02-17 10:59 ` Filipe Manana
2021-02-17 11:28 ` Qu Wenruo
2021-02-17 12:12 ` Filipe Manana
2021-02-17 12:29 ` Qu Wenruo
2021-02-17 12:47 ` Filipe Manana [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAL3q7H6keR5t67W0MEkrYjPA_uQJBjrNjfxgn+vfdZDrhi-gJw@mail.gmail.com \
--to=fdmanana@gmail.com \
--cc=farseerfc@gmail.com \
--cc=linux-btrfs@vger.kernel.org \
--cc=quwenruo.btrfs@gmx.com \
--cc=wqu@suse.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).