That is correct. This bug is present on a single device with duplication
enabled.
Here, in __btrfs_map_block the read length can be reduced to the stripe length
when duplication is enabled.
if (map->type & (BTRFS_BLOCK_GROUP_RAID0 | BTRFS_BLOCK_GROUP_RAID1 |
BTRFS_BLOCK_GROUP_RAID1C3 | BTRFS_BLOCK_GROUP_RAID1C4 |
BTRFS_BLOCK_GROUP_RAID5 | BTRFS_BLOCK_GROUP_RAID6 |
BTRFS_BLOCK_GROUP_RAID10 |
BTRFS_BLOCK_GROUP_DUP)) {
/* we limit the length of each bio to what fits in a stripe */
*length = min_t(u64, ce->size - offset,
map->stripe_len - stripe_offset);
} else {
*length = ce->size - offset;
}
read_extent_data fails to read the complete length. I implemented a patch
similar to yours and it worked perfectly. I am a little sad you beat me to
submitting it.
Thanks,
Sam Winchenbach
-----Original Message-----
From: Qu Wenruo <quwenruo.bt...@gmx.com>
Sent: Friday, December 30, 2022 7:10 PM
To: Sam Winchenbach <swichenb...@tethers.com>; Heinrich Schuchardt
<xypron.g...@gmx.de>; Marek Behún <ka...@kernel.org>
Cc: u-boot@lists.denx.de; Qu Wenruo <w...@suse.com>; linux-bt...@vger.kernel.org
Subject: Re: Possible bug in BTRFS w/ Duplication
On 2022/12/30 23:28, Sam Winchenbach wrote:
> I believe you fixed the issue with the patch you presented. I was in the
> process of testing a similar fix for release and it solved the issue I
> encountered.
But I still want to make sure that only RAID0 on single device can cause the
problem.
For multi-device RAID0/RAID10/RAID5/6, we don't support them until we can scan
all devices in U-boot...
Thanks,
Qu
>
> Thanks,
> Sam Winchenbach
>
> -----Original Message-----
> From: U-Boot <u-boot-boun...@lists.denx.de> On Behalf Of Qu Wenruo
> Sent: Thursday, December 29, 2022 7:01 PM
> To: Heinrich Schuchardt <xypron.g...@gmx.de>; Sam Winchenbach
> <swichenb...@tethers.com>; Marek Behún <ka...@kernel.org>
> Cc: u-boot@lists.denx.de; Qu Wenruo <w...@suse.com>;
> linux-bt...@vger.kernel.org
> Subject: Re: Possible bug in BTRFS w/ Duplication
>
>
>
> On 2022/12/29 22:12, Heinrich Schuchardt wrote:
>> On 12/28/22 21:51, Sam Winchenbach wrote:
>>> Hello,
>>>
>>> Hello, I have hit the following situation when trying to load files
>>> from a BTRFS partition with duplication enabled.
>
> You mean multi-device?
>
> For DUP/RAID1 duplication, they don't have stripe limitation at all.
>
> Thus I believe you're talking about RAID0 (which doesn't have any
> duplication/extra mirrors) or RAID10 or RAID5/6?
>
> But for now, we don't support multi-device in U-boot yet, thus I'm not sure
> what situation you're talking about.
>
> Mind to run the following command?
>
> # btrfs fi usage <mnt of the btrfs>
>
>>>
>>> In the first example I read a 16KiB file - __btrfs_map_block()
>>> changes the length to something larger than the file being read.
>>> This works fine, as length is later clamped to the file size.
>>>
>>> In the second example, __btrfs_map_block() changes the length
>>> parameter to something smaller than the file (the size of a stripe).
>>> This seems to break this check here:
>>>
>>> read = len;
>>> num_copies = btrfs_num_copies(fs_info, logical, len);
>>> for (i = 1; i <= num_copies; i++) {
>>> ret = read_extent_data(fs_info, dest, logical, &read, i);
>>> if (ret < 0 || read != len) {
>>> continue;
>>> }
>>> finished = true;
>>> break;
>>> }
>>>
>>> The problem being that read is always less than len.
>>>
>>> I am not sure if __btrfs_map_block is changing "len" to the
>>> incorrect value, or if there is some logic in "read_extent_data"
>>> that isn't correct. Any pointers on how this code is supposed to
>>> work would be greatly appreciated.
>>> Thanks.
>>
>> Thanks for reporting the issue
>>
>> $ scripts/get_maintainer.pl -f fs/btrfs/volumes.c
>>
>> suggests to include
>>
>> "Marek Behún" <ka...@kernel.org> (maintainer:BTRFS) Qu Wenruo
>> <w...@suse.com> (reviewer:BTRFS) linux-bt...@vger.kernel.org
>>
>> to the communication.
>>
>> Best regards
>>
>> Heinrich
>>
>>>
>>> === EXAMPLE 2 ===
>>> Zynq> load mmc 1:0 0 16K
>>> [btrfs_file_read,fs/btrfs/inode.c:710] === read the aligned part ===
>>> [btrfs_read_extent_reg,fs/btrfs/inode.c:458] before read_extent_data
>>> (ret = 0, read = 16384, len = 16384)
>>> [read_extent_data,fs/btrfs/disk-io.c:547] before __btrfs_map_block
>>> (len = 16384) [read_extent_data,fs/btrfs/disk-io.c:550] after
>>> __btrfs_map_block (len = 28672)
>>> [read_extent_data,fs/btrfs/disk-io.c:565] before __btrfs_devread
>>> (len = 16384) [read_extent_data,fs/btrfs/disk-io.c:568] after
>>> __btrfs_devread (len =
>>> 16384)
>>> [btrfs_read_extent_reg,fs/btrfs/inode.c:460] after read_extent_data
>>> (ret = 0, read = 16384, len = 16384)
>>> cur: 0, extent_num_bytes: 16384, aligned_end: 16384
>>> 16384 bytes read in 100 ms (159.2 KiB/s)
>>>
>>> === EXAMPLE 2 ===
>>> Zynq> load mmc 1:0 0 32K
>>> [btrfs_file_read,fs/btrfs/inode.c:710] === read the aligned part ===
>>> [btrfs_read_extent_reg,fs/btrfs/inode.c:458] before read_extent_data
>>> (ret = 0, read = 32768, len = 32768)
>>> [read_extent_data,fs/btrfs/disk-io.c:547] before __btrfs_map_block
>>> (len = 32768) [read_extent_data,fs/btrfs/disk-io.c:550] after
>>> __btrfs_map_block (len = 12288)
>>> [read_extent_data,fs/btrfs/disk-io.c:565] before __btrfs_devread
>>> (len = 12288) [read_extent_data,fs/btrfs/disk-io.c:568] after
>>> __btrfs_devread (len =
>>> 12288)
>
> So the first 3 sectors are before the stripe boundary and we read it
> correctly.
>
>>> [btrfs_read_extent_reg,fs/btrfs/inode.c:460] after read_extent_data
>>> (ret = 0, read = 12288, len = 32768)
>>> [btrfs_read_extent_reg,fs/btrfs/inode.c:458] before read_extent_data
>>> (ret = 0, read = 12288, len = 32768)
>>> [read_extent_data,fs/btrfs/disk-io.c:547] before __btrfs_map_block
>>> (len = 12288) [read_extent_data,fs/btrfs/disk-io.c:550] after
>>> __btrfs_map_block (len = 12288)
>
> I believe this is the problem.
>
> If we're reading the full 32K, and the first 12K is in the first stripe, we
> should then try to map the remaining 20K, not the 12K again.
>
> I'll look into the situation.
> But if you can provide the image or the dump, it can greatly help the
> debugging.
>
> Thanks,
> Qu
>
>>> [read_extent_data,fs/btrfs/disk-io.c:565] before __btrfs_devread
>>> (len = 12288) [read_extent_data,fs/btrfs/disk-io.c:568] after
>>> __btrfs_devread (len =
>>> 12288)
>>> [btrfs_read_extent_reg,fs/btrfs/inode.c:460] after read_extent_data
>>> (ret = 0, read = 12288, len = 32768)
>>> file: fs/btrfs/inode.c, line: 468
>>> cur: 0, extent_num_bytes: 32768, aligned_end: 32768
>>> -----> btrfs_read_extent_reg: -5, line: 758
>>> BTRFS: An error occurred while reading file 32K Failed to load '32K'
>>>
>>>
>>>
>>>
>>>
>>> Sam Winchenbach
>>> Embedded Software Engineer III
>>> Tethers Unlimited, Inc. | Connect Your Universe | www.tethers.com
>>> swinchenb...@tethers.com | C: 207-974-6934
>>> 11711 North Creek Pkwy # D113, Bothell, WA 98011-8808, USA
>>
