Re: [f2fs-dev] [bug report]WARNING: CPU: 22 PID: 44011 at fs/iomap/iter.c:51 iomap_iter+0x32b observed with blktests zbd/010

[Chao Yu <chao@kernel.org>]

On 2024/3/25 10:14, Shinichiro Kawasaki wrote:
> On Mar 24, 2024 / 20:13, Chao Yu wrote:
> ...
>> Hi Shinichiro,
>>
>> Can you please check below diff? IIUC, for the case: f2fs_map_blocks()
>> returns zero blkaddr in non-primary device, which is a verified valid
>> block address, we'd better to check m_flags & F2FS_MAP_MAPPED instead
>> of map.m_pblk != NULL_ADDR to decide whether tagging IOMAP_MAPPED flag
>> or not.
>>
>> ---
>>   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 6f66e3e4221a..41a56d4298c8 100644
>> --- a/fs/f2fs/data.c
>> +++ b/fs/f2fs/data.c
>> @@ -4203,7 +4203,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;
>>
> 
> Thanks Chao, I confirmed that the diff above avoids the WARN and zbd/010
> failure. From that point of view, it looks good.

Thank you for the confirmation. :)

> 
> One thing I noticed is that the commit message of 8d3c1fa3fa5ea ("f2fs:
> don't rely on F2FS_MAP_* in f2fs_iomap_begin") says that f2fs_map_blocks()
> might be setting F2FS_MAP_* flag on a hole, and that's why the commit
> avoided the F2FS_MAP_MAPPED flag check. So I was not sure if it is the
> right thing to reintroduce the flag check.

I didn't see such logic in previous f2fs_map_blocks(, F2FS_GET_BLOCK_DIO) codebase,
I doubt it hits the same case: map.m_pblk is valid zero blkaddr which locates in
the head of secondary device? What do you think?

Quoted commit message from 8d3c1fa3fa5ea:

When testing with a mixed zoned / convention device combination, there
are regular but not 100% reproducible failures in xfstests generic/113
where the __is_valid_data_blkaddr assert hits due to finding a hole.

Previous code:

-	if (map.m_flags & (F2FS_MAP_MAPPED | F2FS_MAP_UNWRITTEN)) {
-		iomap->length = blks_to_bytes(inode, map.m_len);
-		if (map.m_flags & F2FS_MAP_MAPPED) {
-			iomap->type = IOMAP_MAPPED;
-			iomap->flags |= IOMAP_F_MERGED;
-		} else {
-			iomap->type = IOMAP_UNWRITTEN;
-		}
-		if (WARN_ON_ONCE(!__is_valid_data_blkaddr(map.m_pblk)))
-			return -EINVAL;

Thanks,


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel