I confirmed the issue was fixed now, thanks.
Tested-by: Yi Zhang <yi.zh...@redhat.com>
On Mon, Mar 25, 2024 at 11:26 PM Chao Yu <c...@kernel.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_urgv...@mail.gmail.com/
> [2]
> https://lore.kernel.org/linux-f2fs-devel/gngdj77k4picagsfdtiaa7gpgnup6fsgwzsltx6milmhegmjff@iax2n4wvrqye/
>
> Reported-by: Yi Zhang <yi.zh...@redhat.com>
> Closes:
> https://lore.kernel.org/linux-f2fs-devel/CAHj4cs-kfojYC9i0G73PRkYzcxCTex=-vugrfep40g_urgv...@mail.gmail.com/
> Tested-by: Shin'ichiro Kawasaki <shinichiro.kawas...@wdc.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 <c...@kernel.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
>
--
Best Regards,
Yi Zhang
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
