Re: "bad tree block start" when trying to mount on ARM

From: Qu Wenruo <quwenruo.btrfs@gmx.com>
To: Erik Jensen <erikjensen@rkjnsn.net>
Cc: Su Yue <l@damenly.su>, Hugo Mills <hugo@carfax.org.uk>,
	linux-btrfs <linux-btrfs@vger.kernel.org>
Subject: Re: "bad tree block start" when trying to mount on ARM
Date: Thu, 18 Feb 2021 14:09:11 +0800	[thread overview]
Message-ID: <c6c8cb80-455a-181b-ada5-83001d387044@gmx.com> (raw)
In-Reply-To: <CAMj6ewOze4Ngw7ydj_Ry2nLeyvWs_dB=fuGJ2zBdCEepTiC6yA@mail.gmail.com>



On 2021/2/18 下午1:49, Erik Jensen wrote:
> On Wed, Feb 17, 2021 at 9:24 PM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
>> Got it now.
>>
>> [  295.249182] read_extent_buffer_pages: eb->start=26207780683776 mirror=0
>> [  295.249188] __btrfs_map_block: logical=8615594639360 chunk
>> start=8614760677376 len=4294967296 type=0x81
>> [  295.249189] __btrfs_map_block: stripe[0] devid=3 phy=2118735708160
>>
>> Note that, the initial request is to read from 26207780683776.
>> But inside btrfs_map_block(), we want to read from 8615594639360.
>>
>> This is totally screwed up in a unexpected way.
>>
>> 26207780683776 = 0x17d5f9754000
>> 8615594639360  = 0x07d5f9754000
>>
>> See the missing leading 1, which screws up the result.
>>
>> The problem should be the logical calculation part, which doesn't do
>> proper u64 conversion which could cause the problem.
>>
>> Would you like to test the single line fix below?
>>
>> Thanks,
>> Qu
>>
>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>> index b8fab44394f5..69d728f5ff9e 100644
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -6575,7 +6575,7 @@ blk_status_t btrfs_map_bio(struct btrfs_fs_info
>> *fs_info, struct bio *bio,
>>    {
>>           struct btrfs_device *dev;
>>           struct bio *first_bio = bio;
>> -       u64 logical = bio->bi_iter.bi_sector << 9;
>> +       u64 logical = ((u64)bio->bi_iter.bi_sector) << 9;
>>           u64 length = 0;
>>           u64 map_length;
>>           int ret;
>
> So… it appears my kernel tree (Arch32's 5.10.14-arch1) already has that:
>

And I also noticed that since v5.2 kernel, we should already have
bi_sector as u64.

So why that left shift would get higher bits missing is really strange.
Especially the missing part is just at the 45 bit, not 32 bit boundary.

Then what about this diff? It goes multiplying other than using
dangerous left shift.

(Also, it's recommended to still use previous debug diffs, so if it
doesn't work we still have a chance to know what's going wrong).

Thanks,
Qu

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index b8fab44394f5..15cea408a51f 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -6575,7 +6575,7 @@ blk_status_t btrfs_map_bio(struct btrfs_fs_info
*fs_info, struct bio *bio,
  {
         struct btrfs_device *dev;
         struct bio *first_bio = bio;
-       u64 logical = bio->bi_iter.bi_sector << 9;
+       u64 logical = bio->bi_iter.bi_sector * 512ULL;
         u64 length = 0;
         u64 map_length;
         int ret;



> blk_status_t btrfs_map_bio(struct btrfs_fs_info *fs_info, struct bio *bio,
>                             int mirror_num)
> {
>          struct btrfs_device *dev;
>          struct bio *first_bio = bio;
>          u64 logical = (u64)bio->bi_iter.bi_sector << 9;
>          u64 length = 0;
>          u64 map_length;
>          int ret;
>          int dev_nr;
>          int total_devs;
>          struct btrfs_bio *bbio = NULL;
>
