linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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.”

      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).