Re: [f2fs-dev] [PATCH 3/4] f2fs: fix fiemap boundary handling when read extent cache is incomplete

[Jaegeuk Kim via Linux-f2fs-devel <linux-f2fs-devel@lists.sourceforge.net>]

On 02/03, Yongpeng Yang wrote:
> From: Yongpeng Yang <yangyongpeng@xiaomi.com>
> 
> f2fs_fiemap() calls f2fs_map_blocks() to obtain the block mapping a
> file, and then merges contiguous mappings into extents. If the mapping
> is found in the read extent cache, node blocks do not need to be read.
> However, in the following scenario, a contiguous extent can be split
> into two extents:
> 
> root@vm:/mnt/f2fs# dd if=/dev/zero of=data.4M bs=1M count=4 && sync
> root@vm:/mnt/f2fs# dd if=/dev/zero of=data.4M bs=1M count=2 seek=2 conv=notrunc && sync
> root@vm:/mnt/f2fs# echo 3 > /proc/sys/vm/drop_caches # drop 2M~4M extent cache
> root@vm:/mnt/f2fs# dd if=/dev/zero of=data.4M bs=1M count=2 seek=0 conv=notrunc && sync
> root@vm:/mnt/f2fs# f2fs_io fiemap 0 1024 data.4M
> Fiemap: offset = 0 len = 1024
> logical addr.    physical addr.   length           flags
> 0	0000000000000000 0000000006400000 0000000000200000 00001000
> 1	0000000000200000 0000000006600000 0000000000200000 00001001
> 
> Although the physical addresses of the ranges 0～2MB and 2M～4MB are
> contiguous, the mapping for the 2M～4MB range is not present in memory.
> When the physical addresses for the 0～2MB range are updated, no merge
> happens because the adjacent mapping is missing from the in-memory
> cache. As a result, fiemap reports two separate extents instead of a
> single contiguous one.
> 
> The root cause is that the read extent cache does not guarantee that all
> blocks of an extent are present in memory. Therefore, when the extent
> length returned by f2fs_map_blocks_cached() is smaller than maxblocks,
> the remaining mappings are retrieved via f2fs_get_dnode_of_data() to
> ensure correct fiemap extent boundary handling.
> 
> Fixes: cd8fc5226bef ("f2fs: remove the create argument to f2fs_map_blocks")
> Signed-off-by: Yongpeng Yang <yangyongpeng@xiaomi.com>
> ---
>  fs/f2fs/data.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 42f15fd9c68e..eedadccf86bb 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1623,8 +1623,18 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>  	lfs_dio_write = (flag == F2FS_GET_BLOCK_DIO && f2fs_lfs_mode(sbi) &&
>  				map->m_may_create);
>  
> -	if (!map->m_may_create && f2fs_map_blocks_cached(inode, map, flag))
> -		goto out;
> +	if (!map->m_may_create) {
> +		if (f2fs_map_blocks_cached(inode, map, flag)) {
> +			if (map->m_len == maxblocks)
> +				goto out;
> +			else {
> +				pgofs =	(pgoff_t)map->m_lblk + map->m_len;
> +				end = map->m_lblk + maxblocks;
> +				ofs = map->m_len;
> +				goto next_dnode;
> +			}
> +		}
> +	}

Cleaned up as below:

-       if (!map->m_may_create && f2fs_map_blocks_cached(inode, map, flag))
-               goto out;
+       if (!map->m_may_create && f2fs_map_blocks_cached(inode, map, flag)) {
+               if (map->m_len == maxblocks)
+                       goto out;
+
+               pgofs = (pgoff_t)map->m_lblk + map->m_len;
+               end = map->m_lblk + maxblocks;
+               ofs = map->m_len;
+               goto next_dnode;
+       }

>  
>  	map->m_bdev = inode->i_sb->s_bdev;
>  	map->m_multidev_dio =
> -- 
> 2.43.0


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