[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <a0f95756-aaeb-43d7-93cc-a8befee2ec78@kernel.org>
Date: Tue, 26 Mar 2024 21:43:48 +0800
From: Chao Yu <chao@...nel.org>
To: Yi Zhang <yi.zhang@...hat.com>
Cc: jaegeuk@...nel.org, linux-f2fs-devel@...ts.sourceforge.net,
linux-kernel@...r.kernel.org,
Shin'ichiro Kawasaki <shinichiro.kawasaki@....com>
Subject: Re: [PATCH] f2fs: multidev: fix to recognize valid zero block address
On 2024/3/26 21:26, Yi Zhang wrote:
> I confirmed the issue was fixed now, thanks.
>
> Tested-by: Yi Zhang <yi.zhang@...hat.com>
Yi, thanks for the test.
Thanks,
>
> On Mon, Mar 25, 2024 at 11:26 PM Chao Yu <chao@...nel.org> wrote:
>>
>> As reported by Yi Zhang in mailing list [1], kernel warning was catched
>> during zbd/010 test as below:
>>
>> ./check zbd/010
>> zbd/010 (test gap zone support with F2FS) [failed]
>> runtime ... 3.752s
>> something found in dmesg:
>> [ 4378.146781] run blktests zbd/010 at 2024-02-18 11:31:13
>> [ 4378.192349] null_blk: module loaded
>> [ 4378.209860] null_blk: disk nullb0 created
>> [ 4378.413285] scsi_debug:sdebug_driver_probe: scsi_debug: trim
>> poll_queues to 0. poll_q/nr_hw = (0/1)
>> [ 4378.422334] scsi host15: scsi_debug: version 0191 [20210520]
>> dev_size_mb=1024, opts=0x0, submit_queues=1, statistics=0
>> [ 4378.434922] scsi 15:0:0:0: Direct-Access-ZBC Linux
>> scsi_debug 0191 PQ: 0 ANSI: 7
>> [ 4378.443343] scsi 15:0:0:0: Power-on or device reset occurred
>> [ 4378.449371] sd 15:0:0:0: Attached scsi generic sg5 type 20
>> [ 4378.449418] sd 15:0:0:0: [sdf] Host-managed zoned block device
>> ...
>> (See '/mnt/tests/gitlab.com/api/v4/projects/19168116/repository/archive.zip/storage/blktests/blk/blktests/results/nodev/zbd/010.dmesg'
>>
>> WARNING: CPU: 22 PID: 44011 at fs/iomap/iter.c:51
>> CPU: 22 PID: 44011 Comm: fio Not tainted 6.8.0-rc3+ #1
>> RIP: 0010:iomap_iter+0x32b/0x350
>> Call Trace:
>> <TASK>
>> __iomap_dio_rw+0x1df/0x830
>> f2fs_file_read_iter+0x156/0x3d0 [f2fs]
>> aio_read+0x138/0x210
>> io_submit_one+0x188/0x8c0
>> __x64_sys_io_submit+0x8c/0x1a0
>> do_syscall_64+0x86/0x170
>> entry_SYSCALL_64_after_hwframe+0x6e/0x76
>>
>> Shinichiro Kawasaki helps to analyse this issue and proposes a potential
>> fixing patch in [2].
>>
>> Quoted from reply of Shinichiro Kawasaki:
>>
>> "I confirmed that the trigger commit is dbf8e63f48af as Yi reported. I took a
>> look in the commit, but it looks fine to me. So I thought the cause is not
>> in the commit diff.
>>
>> I found the WARN is printed when the f2fs is set up with multiple devices,
>> and read requests are mapped to the very first block of the second device in the
>> direct read path. In this case, f2fs_map_blocks() and f2fs_map_blocks_cached()
>> modify map->m_pblk as the physical block address from each block device. It
>> becomes zero when it is mapped to the first block of the device. However,
>> f2fs_iomap_begin() assumes that map->m_pblk is the physical block address of the
>> whole f2fs, across the all block devices. It compares map->m_pblk against
>> NULL_ADDR == 0, then go into the unexpected branch and sets the invalid
>> iomap->length. The WARN catches the invalid iomap->length.
>>
>> This WARN is printed even for non-zoned block devices, by following steps.
>>
>> - Create two (non-zoned) null_blk devices memory backed with 128MB size each:
>> nullb0 and nullb1.
>> # mkfs.f2fs /dev/nullb0 -c /dev/nullb1
>> # mount -t f2fs /dev/nullb0 "${mount_dir}"
>> # dd if=/dev/zero of="${mount_dir}/test.dat" bs=1M count=192
>> # dd if="${mount_dir}/test.dat" of=/dev/null bs=1M count=192 iflag=direct
>>
>> ..."
>>
>> So, the root cause of this issue is: when multi-devices feature is on,
>> f2fs_map_blocks() may return zero blkaddr in non-primary device, which is
>> a verified valid block address, however, f2fs_iomap_begin() treats it as
>> an invalid block address, and then it triggers the warning in iomap
>> framework code.
>>
>> Finally, as discussed, we decide to use a more simple and direct way that
>> checking (map.m_flags & F2FS_MAP_MAPPED) condition instead of
>> (map.m_pblk != NULL_ADDR) to fix this issue.
>>
>> Thanks a lot for the effort of Yi Zhang and Shinichiro Kawasaki on this
>> issue.
>>
>> [1] https://lore.kernel.org/linux-f2fs-devel/CAHj4cs-kfojYC9i0G73PRkYzcxCTex=-vugRFeP40g_URGvnfQ@mail.gmail.com/
>> [2] https://lore.kernel.org/linux-f2fs-devel/gngdj77k4picagsfdtiaa7gpgnup6fsgwzsltx6milmhegmjff@iax2n4wvrqye/
>>
>> Reported-by: Yi Zhang <yi.zhang@...hat.com>
>> Closes: https://lore.kernel.org/linux-f2fs-devel/CAHj4cs-kfojYC9i0G73PRkYzcxCTex=-vugRFeP40g_URGvnfQ@mail.gmail.com/
>> Tested-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@....com>
>> Fixes: 1517c1a7a445 ("f2fs: implement iomap operations")
>> Fixes: 8d3c1fa3fa5e ("f2fs: don't rely on F2FS_MAP_* in f2fs_iomap_begin")
>> Signed-off-by: Chao Yu <chao@...nel.org>
>> ---
>> fs/f2fs/data.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>> index d9494b5fc7c1..5ef1874b572a 100644
>> --- a/fs/f2fs/data.c
>> +++ b/fs/f2fs/data.c
>> @@ -4185,7 +4185,7 @@ static int f2fs_iomap_begin(struct inode *inode, loff_t offset, loff_t length,
>> if (WARN_ON_ONCE(map.m_pblk == COMPRESS_ADDR))
>> return -EINVAL;
>>
>> - if (map.m_pblk != NULL_ADDR) {
>> + if (map.m_flags & F2FS_MAP_MAPPED) {
>> iomap->length = blks_to_bytes(inode, map.m_len);
>> iomap->type = IOMAP_MAPPED;
>> iomap->flags |= IOMAP_F_MERGED;
>> --
>> 2.40.1
>>
>
>
Powered by blists - more mailing lists