[f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents

[f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents

[wangzijie]

When the data layout is like this:
dnode1:                     dnode2:
[0]      A                  [0]    NEW_ADDR
[1]      A+1                [1]    0x0
...                         ....
[1016]   A+1016
[1017]   B (B!=A+1017)      [1017] 0x0

We can build this kind of layout by following steps(with i_extra_isize:36):
./f2fs_io write 1 0 1881 rand dsync testfile
./f2fs_io write 1 1881 1 rand buffered testfile
./f2fs_io fallocate 0 7708672 4096 testfile

And when we map first data block in dnode2, we will get wrong extent_info data:
map->m_len = 1
ofs = start_pgofs - map->m_lblk = 1882 - 1881 = 1

ei.fofs = start_pgofs = 1882
ei.len = map->m_len - ofs = 1 - 1 = 0

Fix it by skipping updating this kind of extent info.

Signed-off-by: wangzijie <wangzijie1@honor.com>
---
 fs/f2fs/data.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 7961e0ddf..b8bb71852 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1649,6 +1649,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
 
 		switch (flag) {
 		case F2FS_GET_BLOCK_PRECACHE:
+			if (__is_valid_data_blkaddr(map->m_pblk) &&
+				start_pgofs - map->m_lblk == map->m_len)
+				map->m_flags &= ~F2FS_MAP_MAPPED;
 			goto sync_out;
 		case F2FS_GET_BLOCK_BMAP:
 			map->m_pblk = 0;
-- 
2.25.1



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

[f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents

[wangzijie]

When the data layout is like this:
dnode1:                     dnode2:
[0]      A                  [0]    NEW_ADDR
[1]      A+1                [1]    0x0
...                         ....
[1016]   A+1016
[1017]   B (B!=A+1017)      [1017] 0x0

We can build this kind of layout by following steps(with i_extra_isize:36):
./f2fs_io write 1 0 1881 rand dsync testfile
./f2fs_io write 1 1881 1 rand buffered testfile
./f2fs_io fallocate 0 7708672 4096 testfile

And when we map first data block in dnode2, we will get wrong extent_info data:
map->m_len = 1
ofs = start_pgofs - map->m_lblk = 1882 - 1881 = 1

ei.fofs = start_pgofs = 1882
ei.len = map->m_len - ofs = 1 - 1 = 0

Fix it by skipping updating this kind of extent info.

Signed-off-by: wangzijie <wangzijie1@honor.com>
---
 fs/f2fs/data.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 7961e0ddf..b8bb71852 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1649,6 +1649,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
 
 		switch (flag) {
 		case F2FS_GET_BLOCK_PRECACHE:
+			if (__is_valid_data_blkaddr(map->m_pblk) &&
+				start_pgofs - map->m_lblk == map->m_len)
+				map->m_flags &= ~F2FS_MAP_MAPPED;
 			goto sync_out;
 		case F2FS_GET_BLOCK_BMAP:
 			map->m_pblk = 0;
-- 
2.25.1

[f2fs-dev] [PATCH 2/2] f2fs: fix infinite loop in __insert_extent_tree()

[wangzijie]

When we get wrong extent info data, and look up extent_node in rb tree,
it will cause infinite loop (CONFIG_F2FS_CHECK_FS=n). Avoiding this by
return NULL.

Signed-off-by: wangzijie <wangzijie1@honor.com>
---
 fs/f2fs/extent_cache.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
index 199c1e7a8..6ed6f3d1d 100644
--- a/fs/f2fs/extent_cache.c
+++ b/fs/f2fs/extent_cache.c
@@ -605,6 +605,7 @@ static struct extent_node *__insert_extent_tree(struct f2fs_sb_info *sbi,
 			leftmost = false;
 		} else {
 			f2fs_bug_on(sbi, 1);
+			return NULL;
 		}
 	}
 
-- 
2.25.1



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

[f2fs-dev] [PATCH 2/2] f2fs: fix infinite loop in __insert_extent_tree()

[wangzijie]

When we get wrong extent info data, and look up extent_node in rb tree,
it will cause infinite loop (CONFIG_F2FS_CHECK_FS=n). Avoiding this by
return NULL.

Signed-off-by: wangzijie <wangzijie1@honor.com>
---
 fs/f2fs/extent_cache.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
index 199c1e7a8..6ed6f3d1d 100644
--- a/fs/f2fs/extent_cache.c
+++ b/fs/f2fs/extent_cache.c
@@ -605,6 +605,7 @@ static struct extent_node *__insert_extent_tree(struct f2fs_sb_info *sbi,
 			leftmost = false;
 		} else {
 			f2fs_bug_on(sbi, 1);
+			return NULL;
 		}
 	}
 
-- 
2.25.1

Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents

[Chao Yu via Linux-f2fs-devel]

On 9/10/25 21:58, wangzijie wrote:
> When the data layout is like this:
> dnode1:                     dnode2:
> [0]      A                  [0]    NEW_ADDR
> [1]      A+1                [1]    0x0
> ...                         ....
> [1016]   A+1016
> [1017]   B (B!=A+1017)      [1017] 0x0
> 
> We can build this kind of layout by following steps(with i_extra_isize:36):
> ./f2fs_io write 1 0 1881 rand dsync testfile
> ./f2fs_io write 1 1881 1 rand buffered testfile
> ./f2fs_io fallocate 0 7708672 4096 testfile
> 
> And when we map first data block in dnode2, we will get wrong extent_info data:
> map->m_len = 1
> ofs = start_pgofs - map->m_lblk = 1882 - 1881 = 1
> 
> ei.fofs = start_pgofs = 1882
> ei.len = map->m_len - ofs = 1 - 1 = 0

Hi Zijie,

I tried to reproduce w/ below steps:

f2fs_io write 1 0 1881 rand dsync testfile
f2fs_io write 1 1881 1 rand buffered testfile
f2fs_io fallocate 0 7708672 4096 testfile
umount
mount
f2fs_io precache_extents testfile

         f2fs_io-921     [013] .....  1049.855817: f2fs_lookup_start: dev = (253,16), pino = 3, name:testfile, flags:65537
         f2fs_io-921     [013] .....  1049.855870: f2fs_submit_read_bio: dev = (253,16)/(253,16), rw = READ(), DATA, sector = 139280, size = 4096
         f2fs_io-921     [013] .....  1049.856116: f2fs_submit_folio_bio: dev = (253,16), ino = 1, folio_index = 0x5, oldaddr = 0x5553, newaddr = 0x5553, rw = READ(), type = HOT_NODE
         f2fs_io-921     [013] .....  1049.856147: f2fs_submit_read_bio: dev = (253,16)/(253,16), rw = READ(), NODE, sector = 174744, size = 4096
         f2fs_io-921     [013] .....  1049.856273: f2fs_iget: dev = (253,16), ino = 5, pino = 3, i_mode = 0x81ed, i_size = 7712768, i_nlink = 1, i_blocks = 15080, i_advise = 0x0
         f2fs_io-921     [013] .....  1049.856305: f2fs_lookup_end: dev = (253,16), pino = 3, name:testfile, ino:5, err:0
         f2fs_io-921     [013] .....  1049.856316: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 0, type = Read
         f2fs_io-921     [013] .....  1049.856317: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 0, read_ext_info(fofs: 0, len: 512, blk: 1055744)
         f2fs_io-921     [013] .....  1049.856317: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 0, start blkaddr = 0x101c00, len = 0x200, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
         f2fs_io-921     [013] .....  1049.856318: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 512, type = Read
         f2fs_io-921     [013] .....  1049.856318: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 512, read_ext_info(fofs: 0, len: 0, blk: 0)
         f2fs_io-921     [013] .....  1049.856323: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 5, pgofs = 512, len = 352, blkaddr = 18432, c_len = 0
         f2fs_io-921     [013] .....  1049.856328: f2fs_submit_folio_bio: dev = (253,16), ino = 1, folio_index = 0x6, oldaddr = 0x5556, newaddr = 0x5556, rw = READ(), type = HOT_NODE
         f2fs_io-921     [013] .....  1049.856329: f2fs_submit_read_bio: dev = (253,16)/(253,16), rw = READ(), NODE, sector = 174768, size = 4096
         f2fs_io-921     [021] .....  1049.856968: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 5, pgofs = 864, len = 160, blkaddr = 18784, c_len = 0
         f2fs_io-921     [021] .....  1049.857002: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 512, start blkaddr = 0x4800, len = 0x200, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
         f2fs_io-921     [021] .....  1049.857003: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 1025, type = Read
         f2fs_io-921     [021] .....  1049.857004: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 1025, read_ext_info(fofs: 0, len: 0, blk: 0)
         f2fs_io-921     [021] .....  1049.857010: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 5, pgofs = 1025, len = 511, blkaddr = 19457, c_len = 0
         f2fs_io-921     [021] .....  1049.857011: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 1025, start blkaddr = 0x4c01, len = 0x1ff, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
         f2fs_io-921     [021] .....  1049.857012: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 1537, type = Read
         f2fs_io-921     [021] .....  1049.857012: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 1537, read_ext_info(fofs: 0, len: 0, blk: 0)
         f2fs_io-921     [021] .....  1049.857016: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 5, pgofs = 1537, len = 344, blkaddr = 20993, c_len = 0
         f2fs_io-921     [021] .....  1049.857016: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 1537, start blkaddr = 0x5201, len = 0x158, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
         f2fs_io-921     [021] .....  1049.857017: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 1882, type = Read
         f2fs_io-921     [021] .....  1049.857017: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 1882, read_ext_info(fofs: 0, len: 0, blk: 0)
         f2fs_io-921     [021] .....  1049.857024: f2fs_submit_folio_bio: dev = (253,16), ino = 1, folio_index = 0x7, oldaddr = 0x5555, newaddr = 0x5555, rw = READ(), type = HOT_NODE
         f2fs_io-921     [021] .....  1049.857026: f2fs_submit_read_bio: dev = (253,16)/(253,16), rw = READ(), NODE, sector = 174760, size = 4096
         f2fs_io-921     [021] .....  1049.857156: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 1882, start blkaddr = 0x5201, len = 0x0, flags = 0, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0

It seems f2fs_update_read_extent_tree_range() won't insert a zero-sized extent?
Or am I missing something?

Thanks,

> 
> Fix it by skipping updating this kind of extent info.
> 
> Signed-off-by: wangzijie <wangzijie1@honor.com>
> ---
>  fs/f2fs/data.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 7961e0ddf..b8bb71852 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1649,6 +1649,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>  
>  		switch (flag) {
>  		case F2FS_GET_BLOCK_PRECACHE:
> +			if (__is_valid_data_blkaddr(map->m_pblk) &&
> +				start_pgofs - map->m_lblk == map->m_len)
> +				map->m_flags &= ~F2FS_MAP_MAPPED;
>  			goto sync_out;
>  		case F2FS_GET_BLOCK_BMAP:
>  			map->m_pblk = 0;



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

Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents

[Chao Yu]

On 9/10/25 21:58, wangzijie wrote:
> When the data layout is like this:
> dnode1:                     dnode2:
> [0]      A                  [0]    NEW_ADDR
> [1]      A+1                [1]    0x0
> ...                         ....
> [1016]   A+1016
> [1017]   B (B!=A+1017)      [1017] 0x0
> 
> We can build this kind of layout by following steps(with i_extra_isize:36):
> ./f2fs_io write 1 0 1881 rand dsync testfile
> ./f2fs_io write 1 1881 1 rand buffered testfile
> ./f2fs_io fallocate 0 7708672 4096 testfile
> 
> And when we map first data block in dnode2, we will get wrong extent_info data:
> map->m_len = 1
> ofs = start_pgofs - map->m_lblk = 1882 - 1881 = 1
> 
> ei.fofs = start_pgofs = 1882
> ei.len = map->m_len - ofs = 1 - 1 = 0

Hi Zijie,

I tried to reproduce w/ below steps:

f2fs_io write 1 0 1881 rand dsync testfile
f2fs_io write 1 1881 1 rand buffered testfile
f2fs_io fallocate 0 7708672 4096 testfile
umount
mount
f2fs_io precache_extents testfile

         f2fs_io-921     [013] .....  1049.855817: f2fs_lookup_start: dev = (253,16), pino = 3, name:testfile, flags:65537
         f2fs_io-921     [013] .....  1049.855870: f2fs_submit_read_bio: dev = (253,16)/(253,16), rw = READ(), DATA, sector = 139280, size = 4096
         f2fs_io-921     [013] .....  1049.856116: f2fs_submit_folio_bio: dev = (253,16), ino = 1, folio_index = 0x5, oldaddr = 0x5553, newaddr = 0x5553, rw = READ(), type = HOT_NODE
         f2fs_io-921     [013] .....  1049.856147: f2fs_submit_read_bio: dev = (253,16)/(253,16), rw = READ(), NODE, sector = 174744, size = 4096
         f2fs_io-921     [013] .....  1049.856273: f2fs_iget: dev = (253,16), ino = 5, pino = 3, i_mode = 0x81ed, i_size = 7712768, i_nlink = 1, i_blocks = 15080, i_advise = 0x0
         f2fs_io-921     [013] .....  1049.856305: f2fs_lookup_end: dev = (253,16), pino = 3, name:testfile, ino:5, err:0
         f2fs_io-921     [013] .....  1049.856316: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 0, type = Read
         f2fs_io-921     [013] .....  1049.856317: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 0, read_ext_info(fofs: 0, len: 512, blk: 1055744)
         f2fs_io-921     [013] .....  1049.856317: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 0, start blkaddr = 0x101c00, len = 0x200, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
         f2fs_io-921     [013] .....  1049.856318: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 512, type = Read
         f2fs_io-921     [013] .....  1049.856318: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 512, read_ext_info(fofs: 0, len: 0, blk: 0)
         f2fs_io-921     [013] .....  1049.856323: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 5, pgofs = 512, len = 352, blkaddr = 18432, c_len = 0
         f2fs_io-921     [013] .....  1049.856328: f2fs_submit_folio_bio: dev = (253,16), ino = 1, folio_index = 0x6, oldaddr = 0x5556, newaddr = 0x5556, rw = READ(), type = HOT_NODE
         f2fs_io-921     [013] .....  1049.856329: f2fs_submit_read_bio: dev = (253,16)/(253,16), rw = READ(), NODE, sector = 174768, size = 4096
         f2fs_io-921     [021] .....  1049.856968: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 5, pgofs = 864, len = 160, blkaddr = 18784, c_len = 0
         f2fs_io-921     [021] .....  1049.857002: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 512, start blkaddr = 0x4800, len = 0x200, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
         f2fs_io-921     [021] .....  1049.857003: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 1025, type = Read
         f2fs_io-921     [021] .....  1049.857004: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 1025, read_ext_info(fofs: 0, len: 0, blk: 0)
         f2fs_io-921     [021] .....  1049.857010: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 5, pgofs = 1025, len = 511, blkaddr = 19457, c_len = 0
         f2fs_io-921     [021] .....  1049.857011: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 1025, start blkaddr = 0x4c01, len = 0x1ff, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
         f2fs_io-921     [021] .....  1049.857012: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 1537, type = Read
         f2fs_io-921     [021] .....  1049.857012: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 1537, read_ext_info(fofs: 0, len: 0, blk: 0)
         f2fs_io-921     [021] .....  1049.857016: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 5, pgofs = 1537, len = 344, blkaddr = 20993, c_len = 0
         f2fs_io-921     [021] .....  1049.857016: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 1537, start blkaddr = 0x5201, len = 0x158, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
         f2fs_io-921     [021] .....  1049.857017: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 1882, type = Read
         f2fs_io-921     [021] .....  1049.857017: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 1882, read_ext_info(fofs: 0, len: 0, blk: 0)
         f2fs_io-921     [021] .....  1049.857024: f2fs_submit_folio_bio: dev = (253,16), ino = 1, folio_index = 0x7, oldaddr = 0x5555, newaddr = 0x5555, rw = READ(), type = HOT_NODE
         f2fs_io-921     [021] .....  1049.857026: f2fs_submit_read_bio: dev = (253,16)/(253,16), rw = READ(), NODE, sector = 174760, size = 4096
         f2fs_io-921     [021] .....  1049.857156: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 1882, start blkaddr = 0x5201, len = 0x0, flags = 0, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0

It seems f2fs_update_read_extent_tree_range() won't insert a zero-sized extent?
Or am I missing something?

Thanks,

> 
> Fix it by skipping updating this kind of extent info.
> 
> Signed-off-by: wangzijie <wangzijie1@honor.com>
> ---
>  fs/f2fs/data.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 7961e0ddf..b8bb71852 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1649,6 +1649,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>  
>  		switch (flag) {
>  		case F2FS_GET_BLOCK_PRECACHE:
> +			if (__is_valid_data_blkaddr(map->m_pblk) &&
> +				start_pgofs - map->m_lblk == map->m_len)
> +				map->m_flags &= ~F2FS_MAP_MAPPED;
>  			goto sync_out;
>  		case F2FS_GET_BLOCK_BMAP:
>  			map->m_pblk = 0;

Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents

[wangzijie]

> On 9/10/25 21:58, wangzijie wrote:
> > When the data layout is like this:
> > dnode1:                     dnode2:
> > [0]      A                  [0]    NEW_ADDR
> > [1]      A+1                [1]    0x0
> > ...                         ....
> > [1016]   A+1016
> > [1017]   B (B!=A+1017)      [1017] 0x0
> > 
> > We can build this kind of layout by following steps(with i_extra_isize:36):
> > ./f2fs_io write 1 0 1881 rand dsync testfile
> > ./f2fs_io write 1 1881 1 rand buffered testfile
> > ./f2fs_io fallocate 0 7708672 4096 testfile
> > 
> > And when we map first data block in dnode2, we will get wrong extent_info data:
> > map->m_len = 1
> > ofs = start_pgofs - map->m_lblk = 1882 - 1881 = 1
> > 
> > ei.fofs = start_pgofs = 1882
> > ei.len = map->m_len - ofs = 1 - 1 = 0
> 
> Hi Zijie,
> 
> I tried to reproduce w/ below steps:
> 
> f2fs_io write 1 0 1881 rand dsync testfile
> f2fs_io write 1 1881 1 rand buffered testfile
> f2fs_io fallocate 0 7708672 4096 testfile
> umount
> mount
> f2fs_io precache_extents testfile
> 
>          f2fs_io-921     [013] .....  1049.855817: f2fs_lookup_start: dev = (253,16), pino = 3, name:testfile, flags:65537
>          f2fs_io-921     [013] .....  1049.855870: f2fs_submit_read_bio: dev = (253,16)/(253,16), rw = READ(), DATA, sector = 139280, size = 4096
>          f2fs_io-921     [013] .....  1049.856116: f2fs_submit_folio_bio: dev = (253,16), ino = 1, folio_index = 0x5, oldaddr = 0x5553, newaddr = 0x5553, rw = READ(), type = HOT_NODE
>          f2fs_io-921     [013] .....  1049.856147: f2fs_submit_read_bio: dev = (253,16)/(253,16), rw = READ(), NODE, sector = 174744, size = 4096
>          f2fs_io-921     [013] .....  1049.856273: f2fs_iget: dev = (253,16), ino = 5, pino = 3, i_mode = 0x81ed, i_size = 7712768, i_nlink = 1, i_blocks = 15080, i_advise = 0x0
>          f2fs_io-921     [013] .....  1049.856305: f2fs_lookup_end: dev = (253,16), pino = 3, name:testfile, ino:5, err:0
>          f2fs_io-921     [013] .....  1049.856316: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 0, type = Read
>          f2fs_io-921     [013] .....  1049.856317: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 0, read_ext_info(fofs: 0, len: 512, blk: 1055744)
>          f2fs_io-921     [013] .....  1049.856317: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 0, start blkaddr = 0x101c00, len = 0x200, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
>          f2fs_io-921     [013] .....  1049.856318: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 512, type = Read
>          f2fs_io-921     [013] .....  1049.856318: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 512, read_ext_info(fofs: 0, len: 0, blk: 0)
>          f2fs_io-921     [013] .....  1049.856323: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 5, pgofs = 512, len = 352, blkaddr = 18432, c_len = 0
>          f2fs_io-921     [013] .....  1049.856328: f2fs_submit_folio_bio: dev = (253,16), ino = 1, folio_index = 0x6, oldaddr = 0x5556, newaddr = 0x5556, rw = READ(), type = HOT_NODE
>          f2fs_io-921     [013] .....  1049.856329: f2fs_submit_read_bio: dev = (253,16)/(253,16), rw = READ(), NODE, sector = 174768, size = 4096
>          f2fs_io-921     [021] .....  1049.856968: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 5, pgofs = 864, len = 160, blkaddr = 18784, c_len = 0
>          f2fs_io-921     [021] .....  1049.857002: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 512, start blkaddr = 0x4800, len = 0x200, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
>          f2fs_io-921     [021] .....  1049.857003: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 1025, type = Read
>          f2fs_io-921     [021] .....  1049.857004: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 1025, read_ext_info(fofs: 0, len: 0, blk: 0)
>          f2fs_io-921     [021] .....  1049.857010: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 5, pgofs = 1025, len = 511, blkaddr = 19457, c_len = 0
>          f2fs_io-921     [021] .....  1049.857011: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 1025, start blkaddr = 0x4c01, len = 0x1ff, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
>          f2fs_io-921     [021] .....  1049.857012: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 1537, type = Read
>          f2fs_io-921     [021] .....  1049.857012: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 1537, read_ext_info(fofs: 0, len: 0, blk: 0)
>          f2fs_io-921     [021] .....  1049.857016: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 5, pgofs = 1537, len = 344, blkaddr = 20993, c_len = 0
>          f2fs_io-921     [021] .....  1049.857016: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 1537, start blkaddr = 0x5201, len = 0x158, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
>          f2fs_io-921     [021] .....  1049.857017: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 1882, type = Read
>          f2fs_io-921     [021] .....  1049.857017: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 1882, read_ext_info(fofs: 0, len: 0, blk: 0)
>          f2fs_io-921     [021] .....  1049.857024: f2fs_submit_folio_bio: dev = (253,16), ino = 1, folio_index = 0x7, oldaddr = 0x5555, newaddr = 0x5555, rw = READ(), type = HOT_NODE
>          f2fs_io-921     [021] .....  1049.857026: f2fs_submit_read_bio: dev = (253,16)/(253,16), rw = READ(), NODE, sector = 174760, size = 4096
>          f2fs_io-921     [021] .....  1049.857156: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 1882, start blkaddr = 0x5201, len = 0x0, flags = 0, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
> 
> It seems f2fs_update_read_extent_tree_range() won't insert a zero-sized extent?
> Or am I missing something?
> 
> Thanks,

Hi, Chao
I test it again with below steps:

./f2fs_io write 1 0 1881 rand dsync testfile
./f2fs_io fallocate 0 7708672 4096 testfile
./f2fs_io write 1 1881 1 rand buffered testfile
fsync testfile
umount
mount
./f2fs_io precache_extents testfile

         f2fs_io-8749    [003] .....    86.759281: f2fs_lookup_start: dev = (254,57), pino = 45485, name:testfile, flags:257
         f2fs_io-8749    [003] .....    86.759954: f2fs_iget: dev = (254,57), ino = 501391, pino = 45485, i_mode = 0x81ed, i_size = 7712768, i_nlink = 1, i_blocks = 15080, i_advise = 0x0
         f2fs_io-8749    [003] .....    86.759968: f2fs_lookup_end: dev = (254,57), pino = 45485, name:testfile, ino:501391, err:0
         f2fs_io-8749    [003] .....    86.760000: f2fs_lookup_extent_tree_start: dev = (254,57), ino = 501391, pgofs = 0, type = Read
         f2fs_io-8749    [003] .....    86.760020: f2fs_lookup_read_extent_tree_end: dev = (254,57), ino = 501391, pgofs = 0, read_ext_info(fofs: 0, len: 1881, blk: 3164707)
         f2fs_io-8749    [003] .....    86.760020: f2fs_map_blocks: dev = (254,57), ino = 501391, file offset = 0, start blkaddr = 0x304a23, len = 0x759, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
         f2fs_io-8749    [003] .....    86.760021: f2fs_lookup_extent_tree_start: dev = (254,57), ino = 501391, pgofs = 1881, type = Read
         f2fs_io-8749    [003] .....    86.760022: f2fs_lookup_read_extent_tree_end: dev = (254,57), ino = 501391, pgofs = 1881, read_ext_info(fofs: 0, len: 0, blk: 0)
         f2fs_io-8749    [005] .....    86.760162: f2fs_update_read_extent_tree_range: dev = (254,57), ino = 501391, pgofs = 1881, len = 1, blkaddr = 2688335, c_len = 0
    *****f2fs_io-8749    [005] .....    86.760324: f2fs_update_read_extent_tree_range: dev = (254,57), ino = 501391, pgofs = 1882, len = 0, blkaddr = 2688336, c_len = 0  ******
         f2fs_io-8749    [005] .....    86.760326: f2fs_map_blocks: dev = (254,57), ino = 501391, file offset = 1881, start blkaddr = 0x29054f, len = 0x1, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0


inode:
i_ext: fofs:0 blkaddr:304a23 len:759
i_addr[0x9]     [0x  304a23 : 3164707]
....
i_addr[0x368]   [0x  304d82 : 3165570]

dnode1:
[0]    [0x  304d83 : 3165571]
[1016] [0x  30517b : 3166587]
...
[1017] [0x  29054f : 2688335]

dnode2:
[0]  	NEW_ADDR
[1]     [0x       0 : 0]
...




> > 
> > Fix it by skipping updating this kind of extent info.
> > 
> > Signed-off-by: wangzijie <wangzijie1@honor.com>
> > ---
> >  fs/f2fs/data.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index 7961e0ddf..b8bb71852 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -1649,6 +1649,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
> >  
> >  		switch (flag) {
> >  		case F2FS_GET_BLOCK_PRECACHE:
> > +			if (__is_valid_data_blkaddr(map->m_pblk) &&
> > +				start_pgofs - map->m_lblk == map->m_len)
> > +				map->m_flags &= ~F2FS_MAP_MAPPED;
> >  			goto sync_out;
> >  		case F2FS_GET_BLOCK_BMAP:
> >  			map->m_pblk = 0;



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

Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents

[wangzijie]

> On 9/10/25 21:58, wangzijie wrote:
> > When the data layout is like this:
> > dnode1:                     dnode2:
> > [0]      A                  [0]    NEW_ADDR
> > [1]      A+1                [1]    0x0
> > ...                         ....
> > [1016]   A+1016
> > [1017]   B (B!=A+1017)      [1017] 0x0
> > 
> > We can build this kind of layout by following steps(with i_extra_isize:36):
> > ./f2fs_io write 1 0 1881 rand dsync testfile
> > ./f2fs_io write 1 1881 1 rand buffered testfile
> > ./f2fs_io fallocate 0 7708672 4096 testfile
> > 
> > And when we map first data block in dnode2, we will get wrong extent_info data:
> > map->m_len = 1
> > ofs = start_pgofs - map->m_lblk = 1882 - 1881 = 1
> > 
> > ei.fofs = start_pgofs = 1882
> > ei.len = map->m_len - ofs = 1 - 1 = 0
> 
> Hi Zijie,
> 
> I tried to reproduce w/ below steps:
> 
> f2fs_io write 1 0 1881 rand dsync testfile
> f2fs_io write 1 1881 1 rand buffered testfile
> f2fs_io fallocate 0 7708672 4096 testfile
> umount
> mount
> f2fs_io precache_extents testfile
> 
>          f2fs_io-921     [013] .....  1049.855817: f2fs_lookup_start: dev = (253,16), pino = 3, name:testfile, flags:65537
>          f2fs_io-921     [013] .....  1049.855870: f2fs_submit_read_bio: dev = (253,16)/(253,16), rw = READ(), DATA, sector = 139280, size = 4096
>          f2fs_io-921     [013] .....  1049.856116: f2fs_submit_folio_bio: dev = (253,16), ino = 1, folio_index = 0x5, oldaddr = 0x5553, newaddr = 0x5553, rw = READ(), type = HOT_NODE
>          f2fs_io-921     [013] .....  1049.856147: f2fs_submit_read_bio: dev = (253,16)/(253,16), rw = READ(), NODE, sector = 174744, size = 4096
>          f2fs_io-921     [013] .....  1049.856273: f2fs_iget: dev = (253,16), ino = 5, pino = 3, i_mode = 0x81ed, i_size = 7712768, i_nlink = 1, i_blocks = 15080, i_advise = 0x0
>          f2fs_io-921     [013] .....  1049.856305: f2fs_lookup_end: dev = (253,16), pino = 3, name:testfile, ino:5, err:0
>          f2fs_io-921     [013] .....  1049.856316: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 0, type = Read
>          f2fs_io-921     [013] .....  1049.856317: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 0, read_ext_info(fofs: 0, len: 512, blk: 1055744)
>          f2fs_io-921     [013] .....  1049.856317: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 0, start blkaddr = 0x101c00, len = 0x200, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
>          f2fs_io-921     [013] .....  1049.856318: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 512, type = Read
>          f2fs_io-921     [013] .....  1049.856318: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 512, read_ext_info(fofs: 0, len: 0, blk: 0)
>          f2fs_io-921     [013] .....  1049.856323: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 5, pgofs = 512, len = 352, blkaddr = 18432, c_len = 0
>          f2fs_io-921     [013] .....  1049.856328: f2fs_submit_folio_bio: dev = (253,16), ino = 1, folio_index = 0x6, oldaddr = 0x5556, newaddr = 0x5556, rw = READ(), type = HOT_NODE
>          f2fs_io-921     [013] .....  1049.856329: f2fs_submit_read_bio: dev = (253,16)/(253,16), rw = READ(), NODE, sector = 174768, size = 4096
>          f2fs_io-921     [021] .....  1049.856968: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 5, pgofs = 864, len = 160, blkaddr = 18784, c_len = 0
>          f2fs_io-921     [021] .....  1049.857002: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 512, start blkaddr = 0x4800, len = 0x200, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
>          f2fs_io-921     [021] .....  1049.857003: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 1025, type = Read
>          f2fs_io-921     [021] .....  1049.857004: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 1025, read_ext_info(fofs: 0, len: 0, blk: 0)
>          f2fs_io-921     [021] .....  1049.857010: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 5, pgofs = 1025, len = 511, blkaddr = 19457, c_len = 0
>          f2fs_io-921     [021] .....  1049.857011: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 1025, start blkaddr = 0x4c01, len = 0x1ff, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
>          f2fs_io-921     [021] .....  1049.857012: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 1537, type = Read
>          f2fs_io-921     [021] .....  1049.857012: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 1537, read_ext_info(fofs: 0, len: 0, blk: 0)
>          f2fs_io-921     [021] .....  1049.857016: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 5, pgofs = 1537, len = 344, blkaddr = 20993, c_len = 0
>          f2fs_io-921     [021] .....  1049.857016: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 1537, start blkaddr = 0x5201, len = 0x158, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
>          f2fs_io-921     [021] .....  1049.857017: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 1882, type = Read
>          f2fs_io-921     [021] .....  1049.857017: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 1882, read_ext_info(fofs: 0, len: 0, blk: 0)
>          f2fs_io-921     [021] .....  1049.857024: f2fs_submit_folio_bio: dev = (253,16), ino = 1, folio_index = 0x7, oldaddr = 0x5555, newaddr = 0x5555, rw = READ(), type = HOT_NODE
>          f2fs_io-921     [021] .....  1049.857026: f2fs_submit_read_bio: dev = (253,16)/(253,16), rw = READ(), NODE, sector = 174760, size = 4096
>          f2fs_io-921     [021] .....  1049.857156: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 1882, start blkaddr = 0x5201, len = 0x0, flags = 0, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
> 
> It seems f2fs_update_read_extent_tree_range() won't insert a zero-sized extent?
> Or am I missing something?
> 
> Thanks,

Hi, Chao
I test it again with below steps:

./f2fs_io write 1 0 1881 rand dsync testfile
./f2fs_io fallocate 0 7708672 4096 testfile
./f2fs_io write 1 1881 1 rand buffered testfile
fsync testfile
umount
mount
./f2fs_io precache_extents testfile

         f2fs_io-8749    [003] .....    86.759281: f2fs_lookup_start: dev = (254,57), pino = 45485, name:testfile, flags:257
         f2fs_io-8749    [003] .....    86.759954: f2fs_iget: dev = (254,57), ino = 501391, pino = 45485, i_mode = 0x81ed, i_size = 7712768, i_nlink = 1, i_blocks = 15080, i_advise = 0x0
         f2fs_io-8749    [003] .....    86.759968: f2fs_lookup_end: dev = (254,57), pino = 45485, name:testfile, ino:501391, err:0
         f2fs_io-8749    [003] .....    86.760000: f2fs_lookup_extent_tree_start: dev = (254,57), ino = 501391, pgofs = 0, type = Read
         f2fs_io-8749    [003] .....    86.760020: f2fs_lookup_read_extent_tree_end: dev = (254,57), ino = 501391, pgofs = 0, read_ext_info(fofs: 0, len: 1881, blk: 3164707)
         f2fs_io-8749    [003] .....    86.760020: f2fs_map_blocks: dev = (254,57), ino = 501391, file offset = 0, start blkaddr = 0x304a23, len = 0x759, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
         f2fs_io-8749    [003] .....    86.760021: f2fs_lookup_extent_tree_start: dev = (254,57), ino = 501391, pgofs = 1881, type = Read
         f2fs_io-8749    [003] .....    86.760022: f2fs_lookup_read_extent_tree_end: dev = (254,57), ino = 501391, pgofs = 1881, read_ext_info(fofs: 0, len: 0, blk: 0)
         f2fs_io-8749    [005] .....    86.760162: f2fs_update_read_extent_tree_range: dev = (254,57), ino = 501391, pgofs = 1881, len = 1, blkaddr = 2688335, c_len = 0
    *****f2fs_io-8749    [005] .....    86.760324: f2fs_update_read_extent_tree_range: dev = (254,57), ino = 501391, pgofs = 1882, len = 0, blkaddr = 2688336, c_len = 0  ******
         f2fs_io-8749    [005] .....    86.760326: f2fs_map_blocks: dev = (254,57), ino = 501391, file offset = 1881, start blkaddr = 0x29054f, len = 0x1, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0


inode:
i_ext: fofs:0 blkaddr:304a23 len:759
i_addr[0x9]     [0x  304a23 : 3164707]
....
i_addr[0x368]   [0x  304d82 : 3165570]

dnode1:
[0]    [0x  304d83 : 3165571]
[1016] [0x  30517b : 3166587]
...
[1017] [0x  29054f : 2688335]

dnode2:
[0]  	NEW_ADDR
[1]     [0x       0 : 0]
...




> > 
> > Fix it by skipping updating this kind of extent info.
> > 
> > Signed-off-by: wangzijie <wangzijie1@honor.com>
> > ---
> >  fs/f2fs/data.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index 7961e0ddf..b8bb71852 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -1649,6 +1649,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
> >  
> >  		switch (flag) {
> >  		case F2FS_GET_BLOCK_PRECACHE:
> > +			if (__is_valid_data_blkaddr(map->m_pblk) &&
> > +				start_pgofs - map->m_lblk == map->m_len)
> > +				map->m_flags &= ~F2FS_MAP_MAPPED;
> >  			goto sync_out;
> >  		case F2FS_GET_BLOCK_BMAP:
> >  			map->m_pblk = 0;

Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents

[Chao Yu via Linux-f2fs-devel]

On 9/11/25 14:55, wangzijie wrote:
>> On 9/10/25 21:58, wangzijie wrote:
>>> When the data layout is like this:
>>> dnode1:                     dnode2:
>>> [0]      A                  [0]    NEW_ADDR
>>> [1]      A+1                [1]    0x0
>>> ...                         ....
>>> [1016]   A+1016
>>> [1017]   B (B!=A+1017)      [1017] 0x0
>>>
>>> We can build this kind of layout by following steps(with i_extra_isize:36):
>>> ./f2fs_io write 1 0 1881 rand dsync testfile
>>> ./f2fs_io write 1 1881 1 rand buffered testfile
>>> ./f2fs_io fallocate 0 7708672 4096 testfile
>>>
>>> And when we map first data block in dnode2, we will get wrong extent_info data:
>>> map->m_len = 1
>>> ofs = start_pgofs - map->m_lblk = 1882 - 1881 = 1
>>>
>>> ei.fofs = start_pgofs = 1882
>>> ei.len = map->m_len - ofs = 1 - 1 = 0
>>
>> Hi Zijie,
>>
>> I tried to reproduce w/ below steps:
>>
>> f2fs_io write 1 0 1881 rand dsync testfile
>> f2fs_io write 1 1881 1 rand buffered testfile
>> f2fs_io fallocate 0 7708672 4096 testfile
>> umount
>> mount
>> f2fs_io precache_extents testfile
>>
>>          f2fs_io-921     [013] .....  1049.855817: f2fs_lookup_start: dev = (253,16), pino = 3, name:testfile, flags:65537
>>          f2fs_io-921     [013] .....  1049.855870: f2fs_submit_read_bio: dev = (253,16)/(253,16), rw = READ(), DATA, sector = 139280, size = 4096
>>          f2fs_io-921     [013] .....  1049.856116: f2fs_submit_folio_bio: dev = (253,16), ino = 1, folio_index = 0x5, oldaddr = 0x5553, newaddr = 0x5553, rw = READ(), type = HOT_NODE
>>          f2fs_io-921     [013] .....  1049.856147: f2fs_submit_read_bio: dev = (253,16)/(253,16), rw = READ(), NODE, sector = 174744, size = 4096
>>          f2fs_io-921     [013] .....  1049.856273: f2fs_iget: dev = (253,16), ino = 5, pino = 3, i_mode = 0x81ed, i_size = 7712768, i_nlink = 1, i_blocks = 15080, i_advise = 0x0
>>          f2fs_io-921     [013] .....  1049.856305: f2fs_lookup_end: dev = (253,16), pino = 3, name:testfile, ino:5, err:0
>>          f2fs_io-921     [013] .....  1049.856316: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 0, type = Read
>>          f2fs_io-921     [013] .....  1049.856317: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 0, read_ext_info(fofs: 0, len: 512, blk: 1055744)
>>          f2fs_io-921     [013] .....  1049.856317: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 0, start blkaddr = 0x101c00, len = 0x200, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
>>          f2fs_io-921     [013] .....  1049.856318: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 512, type = Read
>>          f2fs_io-921     [013] .....  1049.856318: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 512, read_ext_info(fofs: 0, len: 0, blk: 0)
>>          f2fs_io-921     [013] .....  1049.856323: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 5, pgofs = 512, len = 352, blkaddr = 18432, c_len = 0
>>          f2fs_io-921     [013] .....  1049.856328: f2fs_submit_folio_bio: dev = (253,16), ino = 1, folio_index = 0x6, oldaddr = 0x5556, newaddr = 0x5556, rw = READ(), type = HOT_NODE
>>          f2fs_io-921     [013] .....  1049.856329: f2fs_submit_read_bio: dev = (253,16)/(253,16), rw = READ(), NODE, sector = 174768, size = 4096
>>          f2fs_io-921     [021] .....  1049.856968: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 5, pgofs = 864, len = 160, blkaddr = 18784, c_len = 0
>>          f2fs_io-921     [021] .....  1049.857002: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 512, start blkaddr = 0x4800, len = 0x200, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
>>          f2fs_io-921     [021] .....  1049.857003: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 1025, type = Read
>>          f2fs_io-921     [021] .....  1049.857004: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 1025, read_ext_info(fofs: 0, len: 0, blk: 0)
>>          f2fs_io-921     [021] .....  1049.857010: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 5, pgofs = 1025, len = 511, blkaddr = 19457, c_len = 0
>>          f2fs_io-921     [021] .....  1049.857011: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 1025, start blkaddr = 0x4c01, len = 0x1ff, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
>>          f2fs_io-921     [021] .....  1049.857012: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 1537, type = Read
>>          f2fs_io-921     [021] .....  1049.857012: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 1537, read_ext_info(fofs: 0, len: 0, blk: 0)
>>          f2fs_io-921     [021] .....  1049.857016: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 5, pgofs = 1537, len = 344, blkaddr = 20993, c_len = 0
>>          f2fs_io-921     [021] .....  1049.857016: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 1537, start blkaddr = 0x5201, len = 0x158, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
>>          f2fs_io-921     [021] .....  1049.857017: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 1882, type = Read
>>          f2fs_io-921     [021] .....  1049.857017: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 1882, read_ext_info(fofs: 0, len: 0, blk: 0)
>>          f2fs_io-921     [021] .....  1049.857024: f2fs_submit_folio_bio: dev = (253,16), ino = 1, folio_index = 0x7, oldaddr = 0x5555, newaddr = 0x5555, rw = READ(), type = HOT_NODE
>>          f2fs_io-921     [021] .....  1049.857026: f2fs_submit_read_bio: dev = (253,16)/(253,16), rw = READ(), NODE, sector = 174760, size = 4096
>>          f2fs_io-921     [021] .....  1049.857156: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 1882, start blkaddr = 0x5201, len = 0x0, flags = 0, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
>>
>> It seems f2fs_update_read_extent_tree_range() won't insert a zero-sized extent?
>> Or am I missing something?
>>
>> Thanks,
> 
> Hi, Chao
> I test it again with below steps:
> 
> ./f2fs_io write 1 0 1881 rand dsync testfile
> ./f2fs_io fallocate 0 7708672 4096 testfile
> ./f2fs_io write 1 1881 1 rand buffered testfile
> fsync testfile
> umount
> mount
> ./f2fs_io precache_extents testfile

Oh, I can reproduce the bug w/ above scripts, thanks.

Thanks,

> 
>          f2fs_io-8749    [003] .....    86.759281: f2fs_lookup_start: dev = (254,57), pino = 45485, name:testfile, flags:257
>          f2fs_io-8749    [003] .....    86.759954: f2fs_iget: dev = (254,57), ino = 501391, pino = 45485, i_mode = 0x81ed, i_size = 7712768, i_nlink = 1, i_blocks = 15080, i_advise = 0x0
>          f2fs_io-8749    [003] .....    86.759968: f2fs_lookup_end: dev = (254,57), pino = 45485, name:testfile, ino:501391, err:0
>          f2fs_io-8749    [003] .....    86.760000: f2fs_lookup_extent_tree_start: dev = (254,57), ino = 501391, pgofs = 0, type = Read
>          f2fs_io-8749    [003] .....    86.760020: f2fs_lookup_read_extent_tree_end: dev = (254,57), ino = 501391, pgofs = 0, read_ext_info(fofs: 0, len: 1881, blk: 3164707)
>          f2fs_io-8749    [003] .....    86.760020: f2fs_map_blocks: dev = (254,57), ino = 501391, file offset = 0, start blkaddr = 0x304a23, len = 0x759, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
>          f2fs_io-8749    [003] .....    86.760021: f2fs_lookup_extent_tree_start: dev = (254,57), ino = 501391, pgofs = 1881, type = Read
>          f2fs_io-8749    [003] .....    86.760022: f2fs_lookup_read_extent_tree_end: dev = (254,57), ino = 501391, pgofs = 1881, read_ext_info(fofs: 0, len: 0, blk: 0)
>          f2fs_io-8749    [005] .....    86.760162: f2fs_update_read_extent_tree_range: dev = (254,57), ino = 501391, pgofs = 1881, len = 1, blkaddr = 2688335, c_len = 0
>     *****f2fs_io-8749    [005] .....    86.760324: f2fs_update_read_extent_tree_range: dev = (254,57), ino = 501391, pgofs = 1882, len = 0, blkaddr = 2688336, c_len = 0  ******
>          f2fs_io-8749    [005] .....    86.760326: f2fs_map_blocks: dev = (254,57), ino = 501391, file offset = 1881, start blkaddr = 0x29054f, len = 0x1, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
> 
> 
> inode:
> i_ext: fofs:0 blkaddr:304a23 len:759
> i_addr[0x9]     [0x  304a23 : 3164707]
> ....
> i_addr[0x368]   [0x  304d82 : 3165570]
> 
> dnode1:
> [0]    [0x  304d83 : 3165571]
> [1016] [0x  30517b : 3166587]
> ...
> [1017] [0x  29054f : 2688335]
> 
> dnode2:
> [0]  	NEW_ADDR
> [1]     [0x       0 : 0]
> ...
> 
> 
> 
> 
>>>
>>> Fix it by skipping updating this kind of extent info.
>>>
>>> Signed-off-by: wangzijie <wangzijie1@honor.com>
>>> ---
>>>  fs/f2fs/data.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>> index 7961e0ddf..b8bb71852 100644
>>> --- a/fs/f2fs/data.c
>>> +++ b/fs/f2fs/data.c
>>> @@ -1649,6 +1649,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>  
>>>  		switch (flag) {
>>>  		case F2FS_GET_BLOCK_PRECACHE:
>>> +			if (__is_valid_data_blkaddr(map->m_pblk) &&
>>> +				start_pgofs - map->m_lblk == map->m_len)
>>> +				map->m_flags &= ~F2FS_MAP_MAPPED;
>>>  			goto sync_out;
>>>  		case F2FS_GET_BLOCK_BMAP:
>>>  			map->m_pblk = 0;
> 



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

Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents

[Chao Yu]

On 9/11/25 14:55, wangzijie wrote:
>> On 9/10/25 21:58, wangzijie wrote:
>>> When the data layout is like this:
>>> dnode1:                     dnode2:
>>> [0]      A                  [0]    NEW_ADDR
>>> [1]      A+1                [1]    0x0
>>> ...                         ....
>>> [1016]   A+1016
>>> [1017]   B (B!=A+1017)      [1017] 0x0
>>>
>>> We can build this kind of layout by following steps(with i_extra_isize:36):
>>> ./f2fs_io write 1 0 1881 rand dsync testfile
>>> ./f2fs_io write 1 1881 1 rand buffered testfile
>>> ./f2fs_io fallocate 0 7708672 4096 testfile
>>>
>>> And when we map first data block in dnode2, we will get wrong extent_info data:
>>> map->m_len = 1
>>> ofs = start_pgofs - map->m_lblk = 1882 - 1881 = 1
>>>
>>> ei.fofs = start_pgofs = 1882
>>> ei.len = map->m_len - ofs = 1 - 1 = 0
>>
>> Hi Zijie,
>>
>> I tried to reproduce w/ below steps:
>>
>> f2fs_io write 1 0 1881 rand dsync testfile
>> f2fs_io write 1 1881 1 rand buffered testfile
>> f2fs_io fallocate 0 7708672 4096 testfile
>> umount
>> mount
>> f2fs_io precache_extents testfile
>>
>>          f2fs_io-921     [013] .....  1049.855817: f2fs_lookup_start: dev = (253,16), pino = 3, name:testfile, flags:65537
>>          f2fs_io-921     [013] .....  1049.855870: f2fs_submit_read_bio: dev = (253,16)/(253,16), rw = READ(), DATA, sector = 139280, size = 4096
>>          f2fs_io-921     [013] .....  1049.856116: f2fs_submit_folio_bio: dev = (253,16), ino = 1, folio_index = 0x5, oldaddr = 0x5553, newaddr = 0x5553, rw = READ(), type = HOT_NODE
>>          f2fs_io-921     [013] .....  1049.856147: f2fs_submit_read_bio: dev = (253,16)/(253,16), rw = READ(), NODE, sector = 174744, size = 4096
>>          f2fs_io-921     [013] .....  1049.856273: f2fs_iget: dev = (253,16), ino = 5, pino = 3, i_mode = 0x81ed, i_size = 7712768, i_nlink = 1, i_blocks = 15080, i_advise = 0x0
>>          f2fs_io-921     [013] .....  1049.856305: f2fs_lookup_end: dev = (253,16), pino = 3, name:testfile, ino:5, err:0
>>          f2fs_io-921     [013] .....  1049.856316: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 0, type = Read
>>          f2fs_io-921     [013] .....  1049.856317: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 0, read_ext_info(fofs: 0, len: 512, blk: 1055744)
>>          f2fs_io-921     [013] .....  1049.856317: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 0, start blkaddr = 0x101c00, len = 0x200, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
>>          f2fs_io-921     [013] .....  1049.856318: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 512, type = Read
>>          f2fs_io-921     [013] .....  1049.856318: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 512, read_ext_info(fofs: 0, len: 0, blk: 0)
>>          f2fs_io-921     [013] .....  1049.856323: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 5, pgofs = 512, len = 352, blkaddr = 18432, c_len = 0
>>          f2fs_io-921     [013] .....  1049.856328: f2fs_submit_folio_bio: dev = (253,16), ino = 1, folio_index = 0x6, oldaddr = 0x5556, newaddr = 0x5556, rw = READ(), type = HOT_NODE
>>          f2fs_io-921     [013] .....  1049.856329: f2fs_submit_read_bio: dev = (253,16)/(253,16), rw = READ(), NODE, sector = 174768, size = 4096
>>          f2fs_io-921     [021] .....  1049.856968: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 5, pgofs = 864, len = 160, blkaddr = 18784, c_len = 0
>>          f2fs_io-921     [021] .....  1049.857002: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 512, start blkaddr = 0x4800, len = 0x200, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
>>          f2fs_io-921     [021] .....  1049.857003: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 1025, type = Read
>>          f2fs_io-921     [021] .....  1049.857004: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 1025, read_ext_info(fofs: 0, len: 0, blk: 0)
>>          f2fs_io-921     [021] .....  1049.857010: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 5, pgofs = 1025, len = 511, blkaddr = 19457, c_len = 0
>>          f2fs_io-921     [021] .....  1049.857011: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 1025, start blkaddr = 0x4c01, len = 0x1ff, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
>>          f2fs_io-921     [021] .....  1049.857012: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 1537, type = Read
>>          f2fs_io-921     [021] .....  1049.857012: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 1537, read_ext_info(fofs: 0, len: 0, blk: 0)
>>          f2fs_io-921     [021] .....  1049.857016: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 5, pgofs = 1537, len = 344, blkaddr = 20993, c_len = 0
>>          f2fs_io-921     [021] .....  1049.857016: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 1537, start blkaddr = 0x5201, len = 0x158, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
>>          f2fs_io-921     [021] .....  1049.857017: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 1882, type = Read
>>          f2fs_io-921     [021] .....  1049.857017: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 1882, read_ext_info(fofs: 0, len: 0, blk: 0)
>>          f2fs_io-921     [021] .....  1049.857024: f2fs_submit_folio_bio: dev = (253,16), ino = 1, folio_index = 0x7, oldaddr = 0x5555, newaddr = 0x5555, rw = READ(), type = HOT_NODE
>>          f2fs_io-921     [021] .....  1049.857026: f2fs_submit_read_bio: dev = (253,16)/(253,16), rw = READ(), NODE, sector = 174760, size = 4096
>>          f2fs_io-921     [021] .....  1049.857156: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 1882, start blkaddr = 0x5201, len = 0x0, flags = 0, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
>>
>> It seems f2fs_update_read_extent_tree_range() won't insert a zero-sized extent?
>> Or am I missing something?
>>
>> Thanks,
> 
> Hi, Chao
> I test it again with below steps:
> 
> ./f2fs_io write 1 0 1881 rand dsync testfile
> ./f2fs_io fallocate 0 7708672 4096 testfile
> ./f2fs_io write 1 1881 1 rand buffered testfile
> fsync testfile
> umount
> mount
> ./f2fs_io precache_extents testfile

Oh, I can reproduce the bug w/ above scripts, thanks.

Thanks,

> 
>          f2fs_io-8749    [003] .....    86.759281: f2fs_lookup_start: dev = (254,57), pino = 45485, name:testfile, flags:257
>          f2fs_io-8749    [003] .....    86.759954: f2fs_iget: dev = (254,57), ino = 501391, pino = 45485, i_mode = 0x81ed, i_size = 7712768, i_nlink = 1, i_blocks = 15080, i_advise = 0x0
>          f2fs_io-8749    [003] .....    86.759968: f2fs_lookup_end: dev = (254,57), pino = 45485, name:testfile, ino:501391, err:0
>          f2fs_io-8749    [003] .....    86.760000: f2fs_lookup_extent_tree_start: dev = (254,57), ino = 501391, pgofs = 0, type = Read
>          f2fs_io-8749    [003] .....    86.760020: f2fs_lookup_read_extent_tree_end: dev = (254,57), ino = 501391, pgofs = 0, read_ext_info(fofs: 0, len: 1881, blk: 3164707)
>          f2fs_io-8749    [003] .....    86.760020: f2fs_map_blocks: dev = (254,57), ino = 501391, file offset = 0, start blkaddr = 0x304a23, len = 0x759, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
>          f2fs_io-8749    [003] .....    86.760021: f2fs_lookup_extent_tree_start: dev = (254,57), ino = 501391, pgofs = 1881, type = Read
>          f2fs_io-8749    [003] .....    86.760022: f2fs_lookup_read_extent_tree_end: dev = (254,57), ino = 501391, pgofs = 1881, read_ext_info(fofs: 0, len: 0, blk: 0)
>          f2fs_io-8749    [005] .....    86.760162: f2fs_update_read_extent_tree_range: dev = (254,57), ino = 501391, pgofs = 1881, len = 1, blkaddr = 2688335, c_len = 0
>     *****f2fs_io-8749    [005] .....    86.760324: f2fs_update_read_extent_tree_range: dev = (254,57), ino = 501391, pgofs = 1882, len = 0, blkaddr = 2688336, c_len = 0  ******
>          f2fs_io-8749    [005] .....    86.760326: f2fs_map_blocks: dev = (254,57), ino = 501391, file offset = 1881, start blkaddr = 0x29054f, len = 0x1, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
> 
> 
> inode:
> i_ext: fofs:0 blkaddr:304a23 len:759
> i_addr[0x9]     [0x  304a23 : 3164707]
> ....
> i_addr[0x368]   [0x  304d82 : 3165570]
> 
> dnode1:
> [0]    [0x  304d83 : 3165571]
> [1016] [0x  30517b : 3166587]
> ...
> [1017] [0x  29054f : 2688335]
> 
> dnode2:
> [0]  	NEW_ADDR
> [1]     [0x       0 : 0]
> ...
> 
> 
> 
> 
>>>
>>> Fix it by skipping updating this kind of extent info.
>>>
>>> Signed-off-by: wangzijie <wangzijie1@honor.com>
>>> ---
>>>  fs/f2fs/data.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>> index 7961e0ddf..b8bb71852 100644
>>> --- a/fs/f2fs/data.c
>>> +++ b/fs/f2fs/data.c
>>> @@ -1649,6 +1649,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>  
>>>  		switch (flag) {
>>>  		case F2FS_GET_BLOCK_PRECACHE:
>>> +			if (__is_valid_data_blkaddr(map->m_pblk) &&
>>> +				start_pgofs - map->m_lblk == map->m_len)
>>> +				map->m_flags &= ~F2FS_MAP_MAPPED;
>>>  			goto sync_out;
>>>  		case F2FS_GET_BLOCK_BMAP:
>>>  			map->m_pblk = 0;
> 

Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents

[wangzijie]

> On 9/10/25 21:58, wangzijie wrote:
> > When the data layout is like this:
> > dnode1:                     dnode2:
> > [0]      A                  [0]    NEW_ADDR
> > [1]      A+1                [1]    0x0
> > ...                         ....
> > [1016]   A+1016
> > [1017]   B (B!=A+1017)      [1017] 0x0
> > 
> > We can build this kind of layout by following steps(with i_extra_isize:36):
> > ./f2fs_io write 1 0 1881 rand dsync testfile
> > ./f2fs_io write 1 1881 1 rand buffered testfile
> > ./f2fs_io fallocate 0 7708672 4096 testfile
> > 
> > And when we map first data block in dnode2, we will get wrong extent_info data:
> > map->m_len = 1
> > ofs = start_pgofs - map->m_lblk = 1882 - 1881 = 1
> > 
> > ei.fofs = start_pgofs = 1882
> > ei.len = map->m_len - ofs = 1 - 1 = 0
> 
> Hi Zijie,
> 
> I tried to reproduce w/ below steps:
> 
> f2fs_io write 1 0 1881 rand dsync testfile
> f2fs_io write 1 1881 1 rand buffered testfile
> f2fs_io fallocate 0 7708672 4096 testfile
> umount
> mount
> f2fs_io precache_extents testfile
> 
>          f2fs_io-921     [013] .....  1049.855817: f2fs_lookup_start: dev = (253,16), pino = 3, name:testfile, flags:65537
>          f2fs_io-921     [013] .....  1049.855870: f2fs_submit_read_bio: dev = (253,16)/(253,16), rw = READ(), DATA, sector = 139280, size = 4096
>          f2fs_io-921     [013] .....  1049.856116: f2fs_submit_folio_bio: dev = (253,16), ino = 1, folio_index = 0x5, oldaddr = 0x5553, newaddr = 0x5553, rw = READ(), type = HOT_NODE
>          f2fs_io-921     [013] .....  1049.856147: f2fs_submit_read_bio: dev = (253,16)/(253,16), rw = READ(), NODE, sector = 174744, size = 4096
>          f2fs_io-921     [013] .....  1049.856273: f2fs_iget: dev = (253,16), ino = 5, pino = 3, i_mode = 0x81ed, i_size = 7712768, i_nlink = 1, i_blocks = 15080, i_advise = 0x0
>          f2fs_io-921     [013] .....  1049.856305: f2fs_lookup_end: dev = (253,16), pino = 3, name:testfile, ino:5, err:0
>          f2fs_io-921     [013] .....  1049.856316: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 0, type = Read
>          f2fs_io-921     [013] .....  1049.856317: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 0, read_ext_info(fofs: 0, len: 512, blk: 1055744)
>          f2fs_io-921     [013] .....  1049.856317: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 0, start blkaddr = 0x101c00, len = 0x200, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
>          f2fs_io-921     [013] .....  1049.856318: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 512, type = Read
>          f2fs_io-921     [013] .....  1049.856318: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 512, read_ext_info(fofs: 0, len: 0, blk: 0)
>          f2fs_io-921     [013] .....  1049.856323: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 5, pgofs = 512, len = 352, blkaddr = 18432, c_len = 0
>          f2fs_io-921     [013] .....  1049.856328: f2fs_submit_folio_bio: dev = (253,16), ino = 1, folio_index = 0x6, oldaddr = 0x5556, newaddr = 0x5556, rw = READ(), type = HOT_NODE
>          f2fs_io-921     [013] .....  1049.856329: f2fs_submit_read_bio: dev = (253,16)/(253,16), rw = READ(), NODE, sector = 174768, size = 4096
>          f2fs_io-921     [021] .....  1049.856968: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 5, pgofs = 864, len = 160, blkaddr = 18784, c_len = 0
>          f2fs_io-921     [021] .....  1049.857002: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 512, start blkaddr = 0x4800, len = 0x200, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
>          f2fs_io-921     [021] .....  1049.857003: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 1025, type = Read
>          f2fs_io-921     [021] .....  1049.857004: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 1025, read_ext_info(fofs: 0, len: 0, blk: 0)
>          f2fs_io-921     [021] .....  1049.857010: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 5, pgofs = 1025, len = 511, blkaddr = 19457, c_len = 0
>          f2fs_io-921     [021] .....  1049.857011: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 1025, start blkaddr = 0x4c01, len = 0x1ff, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
>          f2fs_io-921     [021] .....  1049.857012: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 1537, type = Read
>          f2fs_io-921     [021] .....  1049.857012: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 1537, read_ext_info(fofs: 0, len: 0, blk: 0)
>          f2fs_io-921     [021] .....  1049.857016: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 5, pgofs = 1537, len = 344, blkaddr = 20993, c_len = 0
>          f2fs_io-921     [021] .....  1049.857016: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 1537, start blkaddr = 0x5201, len = 0x158, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
>          f2fs_io-921     [021] .....  1049.857017: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 1882, type = Read
>          f2fs_io-921     [021] .....  1049.857017: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 1882, read_ext_info(fofs: 0, len: 0, blk: 0)
>          f2fs_io-921     [021] .....  1049.857024: f2fs_submit_folio_bio: dev = (253,16), ino = 1, folio_index = 0x7, oldaddr = 0x5555, newaddr = 0x5555, rw = READ(), type = HOT_NODE
>          f2fs_io-921     [021] .....  1049.857026: f2fs_submit_read_bio: dev = (253,16)/(253,16), rw = READ(), NODE, sector = 174760, size = 4096
>          f2fs_io-921     [021] .....  1049.857156: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 1882, start blkaddr = 0x5201, len = 0x0, flags = 0, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
> 
> It seems f2fs_update_read_extent_tree_range() won't insert a zero-sized extent?
> Or am I missing something?
> 
> Thanks,

From the trace, it seems that the data layout is not like what I described?

> > 
> > Fix it by skipping updating this kind of extent info.
> > 
> > Signed-off-by: wangzijie <wangzijie1@honor.com>
> > ---
> >  fs/f2fs/data.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index 7961e0ddf..b8bb71852 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -1649,6 +1649,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
> >  
> >  		switch (flag) {
> >  		case F2FS_GET_BLOCK_PRECACHE:
> > +			if (__is_valid_data_blkaddr(map->m_pblk) &&
> > +				start_pgofs - map->m_lblk == map->m_len)
> > +				map->m_flags &= ~F2FS_MAP_MAPPED;
> >  			goto sync_out;
> >  		case F2FS_GET_BLOCK_BMAP:
> >  			map->m_pblk = 0;




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

Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents

[wangzijie]

> On 9/10/25 21:58, wangzijie wrote:
> > When the data layout is like this:
> > dnode1:                     dnode2:
> > [0]      A                  [0]    NEW_ADDR
> > [1]      A+1                [1]    0x0
> > ...                         ....
> > [1016]   A+1016
> > [1017]   B (B!=A+1017)      [1017] 0x0
> > 
> > We can build this kind of layout by following steps(with i_extra_isize:36):
> > ./f2fs_io write 1 0 1881 rand dsync testfile
> > ./f2fs_io write 1 1881 1 rand buffered testfile
> > ./f2fs_io fallocate 0 7708672 4096 testfile
> > 
> > And when we map first data block in dnode2, we will get wrong extent_info data:
> > map->m_len = 1
> > ofs = start_pgofs - map->m_lblk = 1882 - 1881 = 1
> > 
> > ei.fofs = start_pgofs = 1882
> > ei.len = map->m_len - ofs = 1 - 1 = 0
> 
> Hi Zijie,
> 
> I tried to reproduce w/ below steps:
> 
> f2fs_io write 1 0 1881 rand dsync testfile
> f2fs_io write 1 1881 1 rand buffered testfile
> f2fs_io fallocate 0 7708672 4096 testfile
> umount
> mount
> f2fs_io precache_extents testfile
> 
>          f2fs_io-921     [013] .....  1049.855817: f2fs_lookup_start: dev = (253,16), pino = 3, name:testfile, flags:65537
>          f2fs_io-921     [013] .....  1049.855870: f2fs_submit_read_bio: dev = (253,16)/(253,16), rw = READ(), DATA, sector = 139280, size = 4096
>          f2fs_io-921     [013] .....  1049.856116: f2fs_submit_folio_bio: dev = (253,16), ino = 1, folio_index = 0x5, oldaddr = 0x5553, newaddr = 0x5553, rw = READ(), type = HOT_NODE
>          f2fs_io-921     [013] .....  1049.856147: f2fs_submit_read_bio: dev = (253,16)/(253,16), rw = READ(), NODE, sector = 174744, size = 4096
>          f2fs_io-921     [013] .....  1049.856273: f2fs_iget: dev = (253,16), ino = 5, pino = 3, i_mode = 0x81ed, i_size = 7712768, i_nlink = 1, i_blocks = 15080, i_advise = 0x0
>          f2fs_io-921     [013] .....  1049.856305: f2fs_lookup_end: dev = (253,16), pino = 3, name:testfile, ino:5, err:0
>          f2fs_io-921     [013] .....  1049.856316: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 0, type = Read
>          f2fs_io-921     [013] .....  1049.856317: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 0, read_ext_info(fofs: 0, len: 512, blk: 1055744)
>          f2fs_io-921     [013] .....  1049.856317: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 0, start blkaddr = 0x101c00, len = 0x200, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
>          f2fs_io-921     [013] .....  1049.856318: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 512, type = Read
>          f2fs_io-921     [013] .....  1049.856318: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 512, read_ext_info(fofs: 0, len: 0, blk: 0)
>          f2fs_io-921     [013] .....  1049.856323: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 5, pgofs = 512, len = 352, blkaddr = 18432, c_len = 0
>          f2fs_io-921     [013] .....  1049.856328: f2fs_submit_folio_bio: dev = (253,16), ino = 1, folio_index = 0x6, oldaddr = 0x5556, newaddr = 0x5556, rw = READ(), type = HOT_NODE
>          f2fs_io-921     [013] .....  1049.856329: f2fs_submit_read_bio: dev = (253,16)/(253,16), rw = READ(), NODE, sector = 174768, size = 4096
>          f2fs_io-921     [021] .....  1049.856968: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 5, pgofs = 864, len = 160, blkaddr = 18784, c_len = 0
>          f2fs_io-921     [021] .....  1049.857002: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 512, start blkaddr = 0x4800, len = 0x200, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
>          f2fs_io-921     [021] .....  1049.857003: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 1025, type = Read
>          f2fs_io-921     [021] .....  1049.857004: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 1025, read_ext_info(fofs: 0, len: 0, blk: 0)
>          f2fs_io-921     [021] .....  1049.857010: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 5, pgofs = 1025, len = 511, blkaddr = 19457, c_len = 0
>          f2fs_io-921     [021] .....  1049.857011: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 1025, start blkaddr = 0x4c01, len = 0x1ff, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
>          f2fs_io-921     [021] .....  1049.857012: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 1537, type = Read
>          f2fs_io-921     [021] .....  1049.857012: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 1537, read_ext_info(fofs: 0, len: 0, blk: 0)
>          f2fs_io-921     [021] .....  1049.857016: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 5, pgofs = 1537, len = 344, blkaddr = 20993, c_len = 0
>          f2fs_io-921     [021] .....  1049.857016: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 1537, start blkaddr = 0x5201, len = 0x158, flags = 2, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
>          f2fs_io-921     [021] .....  1049.857017: f2fs_lookup_extent_tree_start: dev = (253,16), ino = 5, pgofs = 1882, type = Read
>          f2fs_io-921     [021] .....  1049.857017: f2fs_lookup_read_extent_tree_end: dev = (253,16), ino = 5, pgofs = 1882, read_ext_info(fofs: 0, len: 0, blk: 0)
>          f2fs_io-921     [021] .....  1049.857024: f2fs_submit_folio_bio: dev = (253,16), ino = 1, folio_index = 0x7, oldaddr = 0x5555, newaddr = 0x5555, rw = READ(), type = HOT_NODE
>          f2fs_io-921     [021] .....  1049.857026: f2fs_submit_read_bio: dev = (253,16)/(253,16), rw = READ(), NODE, sector = 174760, size = 4096
>          f2fs_io-921     [021] .....  1049.857156: f2fs_map_blocks: dev = (253,16), ino = 5, file offset = 1882, start blkaddr = 0x5201, len = 0x0, flags = 0, seg_type = 8, may_create = 0, multidevice = 0, flag = 6, err = 0
> 
> It seems f2fs_update_read_extent_tree_range() won't insert a zero-sized extent?
> Or am I missing something?
> 
> Thanks,

From the trace, it seems that the data layout is not like what I described?

> > 
> > Fix it by skipping updating this kind of extent info.
> > 
> > Signed-off-by: wangzijie <wangzijie1@honor.com>
> > ---
> >  fs/f2fs/data.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index 7961e0ddf..b8bb71852 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -1649,6 +1649,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
> >  
> >  		switch (flag) {
> >  		case F2FS_GET_BLOCK_PRECACHE:
> > +			if (__is_valid_data_blkaddr(map->m_pblk) &&
> > +				start_pgofs - map->m_lblk == map->m_len)
> > +				map->m_flags &= ~F2FS_MAP_MAPPED;
> >  			goto sync_out;
> >  		case F2FS_GET_BLOCK_BMAP:
> >  			map->m_pblk = 0;

Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents

[Chao Yu via Linux-f2fs-devel]

On 9/10/25 21:58, wangzijie wrote:
> When the data layout is like this:
> dnode1:                     dnode2:
> [0]      A                  [0]    NEW_ADDR
> [1]      A+1                [1]    0x0
> ...                         ....
> [1016]   A+1016
> [1017]   B (B!=A+1017)      [1017] 0x0
> 
> We can build this kind of layout by following steps(with i_extra_isize:36):
> ./f2fs_io write 1 0 1881 rand dsync testfile
> ./f2fs_io write 1 1881 1 rand buffered testfile
> ./f2fs_io fallocate 0 7708672 4096 testfile
> 
> And when we map first data block in dnode2, we will get wrong extent_info data:
> map->m_len = 1
> ofs = start_pgofs - map->m_lblk = 1882 - 1881 = 1
> 
> ei.fofs = start_pgofs = 1882
> ei.len = map->m_len - ofs = 1 - 1 = 0
> 
> Fix it by skipping updating this kind of extent info.
> 
> Signed-off-by: wangzijie <wangzijie1@honor.com>
> ---
>  fs/f2fs/data.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 7961e0ddf..b8bb71852 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1649,6 +1649,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>  
>  		switch (flag) {
>  		case F2FS_GET_BLOCK_PRECACHE:
> +			if (__is_valid_data_blkaddr(map->m_pblk) &&
> +				start_pgofs - map->m_lblk == map->m_len)
> +				map->m_flags &= ~F2FS_MAP_MAPPED;

It looks we missed to reset value for map variable in f2fs_precache_extents(),
what do you think of this?

---
 fs/f2fs/file.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 1aae4361d0a8..2b14151d4130 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -3599,7 +3599,7 @@ static int f2fs_ioc_io_prio(struct file *filp, unsigned long arg)
 int f2fs_precache_extents(struct inode *inode)
 {
 	struct f2fs_inode_info *fi = F2FS_I(inode);
-	struct f2fs_map_blocks map;
+	struct f2fs_map_blocks map = { 0 };
 	pgoff_t m_next_extent;
 	loff_t end;
 	int err;
@@ -3617,6 +3617,8 @@ int f2fs_precache_extents(struct inode *inode)

 	while (map.m_lblk < end) {
 		map.m_len = end - map.m_lblk;
+		map.m_pblk = 0;
+		map.m_flags = 0;

 		f2fs_down_write(&fi->i_gc_rwsem[WRITE]);
 		err = f2fs_map_blocks(inode, &map, F2FS_GET_BLOCK_PRECACHE);
-- 
2.49.0

Thanks,

>  			goto sync_out;
>  		case F2FS_GET_BLOCK_BMAP:
>  			map->m_pblk = 0;



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

Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents

[Chao Yu]

On 9/10/25 21:58, wangzijie wrote:
> When the data layout is like this:
> dnode1:                     dnode2:
> [0]      A                  [0]    NEW_ADDR
> [1]      A+1                [1]    0x0
> ...                         ....
> [1016]   A+1016
> [1017]   B (B!=A+1017)      [1017] 0x0
> 
> We can build this kind of layout by following steps(with i_extra_isize:36):
> ./f2fs_io write 1 0 1881 rand dsync testfile
> ./f2fs_io write 1 1881 1 rand buffered testfile
> ./f2fs_io fallocate 0 7708672 4096 testfile
> 
> And when we map first data block in dnode2, we will get wrong extent_info data:
> map->m_len = 1
> ofs = start_pgofs - map->m_lblk = 1882 - 1881 = 1
> 
> ei.fofs = start_pgofs = 1882
> ei.len = map->m_len - ofs = 1 - 1 = 0
> 
> Fix it by skipping updating this kind of extent info.
> 
> Signed-off-by: wangzijie <wangzijie1@honor.com>
> ---
>  fs/f2fs/data.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 7961e0ddf..b8bb71852 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1649,6 +1649,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>  
>  		switch (flag) {
>  		case F2FS_GET_BLOCK_PRECACHE:
> +			if (__is_valid_data_blkaddr(map->m_pblk) &&
> +				start_pgofs - map->m_lblk == map->m_len)
> +				map->m_flags &= ~F2FS_MAP_MAPPED;

It looks we missed to reset value for map variable in f2fs_precache_extents(),
what do you think of this?

---
 fs/f2fs/file.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 1aae4361d0a8..2b14151d4130 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -3599,7 +3599,7 @@ static int f2fs_ioc_io_prio(struct file *filp, unsigned long arg)
 int f2fs_precache_extents(struct inode *inode)
 {
 	struct f2fs_inode_info *fi = F2FS_I(inode);
-	struct f2fs_map_blocks map;
+	struct f2fs_map_blocks map = { 0 };
 	pgoff_t m_next_extent;
 	loff_t end;
 	int err;
@@ -3617,6 +3617,8 @@ int f2fs_precache_extents(struct inode *inode)

 	while (map.m_lblk < end) {
 		map.m_len = end - map.m_lblk;
+		map.m_pblk = 0;
+		map.m_flags = 0;

 		f2fs_down_write(&fi->i_gc_rwsem[WRITE]);
 		err = f2fs_map_blocks(inode, &map, F2FS_GET_BLOCK_PRECACHE);
-- 
2.49.0

Thanks,

>  			goto sync_out;
>  		case F2FS_GET_BLOCK_BMAP:
>  			map->m_pblk = 0;

Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents

[wangzijie]

> On 9/10/25 21:58, wangzijie wrote:
> > When the data layout is like this:
> > dnode1:                     dnode2:
> > [0]      A                  [0]    NEW_ADDR
> > [1]      A+1                [1]    0x0
> > ...                         ....
> > [1016]   A+1016
> > [1017]   B (B!=A+1017)      [1017] 0x0
> > 
> > We can build this kind of layout by following steps(with i_extra_isize:36):
> > ./f2fs_io write 1 0 1881 rand dsync testfile
> > ./f2fs_io write 1 1881 1 rand buffered testfile
> > ./f2fs_io fallocate 0 7708672 4096 testfile
> > 
> > And when we map first data block in dnode2, we will get wrong extent_info data:
> > map->m_len = 1
> > ofs = start_pgofs - map->m_lblk = 1882 - 1881 = 1
> > 
> > ei.fofs = start_pgofs = 1882
> > ei.len = map->m_len - ofs = 1 - 1 = 0
> > 
> > Fix it by skipping updating this kind of extent info.
> > 
> > Signed-off-by: wangzijie <wangzijie1@honor.com>
> > ---
> >  fs/f2fs/data.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index 7961e0ddf..b8bb71852 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -1649,6 +1649,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
> >  
> >  		switch (flag) {
> >  		case F2FS_GET_BLOCK_PRECACHE:
> > +			if (__is_valid_data_blkaddr(map->m_pblk) &&
> > +				start_pgofs - map->m_lblk == map->m_len)
> > +				map->m_flags &= ~F2FS_MAP_MAPPED;
> 
> It looks we missed to reset value for map variable in f2fs_precache_extents(),
> what do you think of this?
> 
> ---
>  fs/f2fs/file.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 1aae4361d0a8..2b14151d4130 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -3599,7 +3599,7 @@ static int f2fs_ioc_io_prio(struct file *filp, unsigned long arg)
>  int f2fs_precache_extents(struct inode *inode)
>  {
>  	struct f2fs_inode_info *fi = F2FS_I(inode);
> -	struct f2fs_map_blocks map;
> +	struct f2fs_map_blocks map = { 0 };
>  	pgoff_t m_next_extent;
>  	loff_t end;
>  	int err;
> @@ -3617,6 +3617,8 @@ int f2fs_precache_extents(struct inode *inode)
> 
>  	while (map.m_lblk < end) {
>  		map.m_len = end - map.m_lblk;
> +		map.m_pblk = 0;
> +		map.m_flags = 0;
> 
>  		f2fs_down_write(&fi->i_gc_rwsem[WRITE]);
>  		err = f2fs_map_blocks(inode, &map, F2FS_GET_BLOCK_PRECACHE);
> -- 
> 2.49.0
> 
> Thanks,
> 
> >  			goto sync_out;
> >  		case F2FS_GET_BLOCK_BMAP:
> >  			map->m_pblk = 0;


We have already reset m_flags (map->m_flags = 0) in f2fs_map_blocks().

I think that this bug is caused by we missed to reset m_flags when we
goto next_dnode in below case：

Data layout is something like this:
dnode1:                     dnode2:
[0]      A                  [0]    NEW_ADDR
[1]      A+1                [1]    0x0
...
[1016]   A+1016
[1017]   B (B!=A+1017)      [1017] 0x0

we map the last block(valid blkaddr) in dnode1:
map->m_flags |= F2FS_MAP_MAPPED;
map->m_pblk = blkaddr(valid blkaddr);
map->m_len = 1;
then we goto next_dnode, meet the first block in dnode2(hole), goto sync_out:
map->m_flags & F2FS_MAP_MAPPED == true, and we make wrong blkaddr/len for extent_info.


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

Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents

[wangzijie]

> On 9/10/25 21:58, wangzijie wrote:
> > When the data layout is like this:
> > dnode1:                     dnode2:
> > [0]      A                  [0]    NEW_ADDR
> > [1]      A+1                [1]    0x0
> > ...                         ....
> > [1016]   A+1016
> > [1017]   B (B!=A+1017)      [1017] 0x0
> > 
> > We can build this kind of layout by following steps(with i_extra_isize:36):
> > ./f2fs_io write 1 0 1881 rand dsync testfile
> > ./f2fs_io write 1 1881 1 rand buffered testfile
> > ./f2fs_io fallocate 0 7708672 4096 testfile
> > 
> > And when we map first data block in dnode2, we will get wrong extent_info data:
> > map->m_len = 1
> > ofs = start_pgofs - map->m_lblk = 1882 - 1881 = 1
> > 
> > ei.fofs = start_pgofs = 1882
> > ei.len = map->m_len - ofs = 1 - 1 = 0
> > 
> > Fix it by skipping updating this kind of extent info.
> > 
> > Signed-off-by: wangzijie <wangzijie1@honor.com>
> > ---
> >  fs/f2fs/data.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index 7961e0ddf..b8bb71852 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -1649,6 +1649,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
> >  
> >  		switch (flag) {
> >  		case F2FS_GET_BLOCK_PRECACHE:
> > +			if (__is_valid_data_blkaddr(map->m_pblk) &&
> > +				start_pgofs - map->m_lblk == map->m_len)
> > +				map->m_flags &= ~F2FS_MAP_MAPPED;
> 
> It looks we missed to reset value for map variable in f2fs_precache_extents(),
> what do you think of this?
> 
> ---
>  fs/f2fs/file.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index 1aae4361d0a8..2b14151d4130 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -3599,7 +3599,7 @@ static int f2fs_ioc_io_prio(struct file *filp, unsigned long arg)
>  int f2fs_precache_extents(struct inode *inode)
>  {
>  	struct f2fs_inode_info *fi = F2FS_I(inode);
> -	struct f2fs_map_blocks map;
> +	struct f2fs_map_blocks map = { 0 };
>  	pgoff_t m_next_extent;
>  	loff_t end;
>  	int err;
> @@ -3617,6 +3617,8 @@ int f2fs_precache_extents(struct inode *inode)
> 
>  	while (map.m_lblk < end) {
>  		map.m_len = end - map.m_lblk;
> +		map.m_pblk = 0;
> +		map.m_flags = 0;
> 
>  		f2fs_down_write(&fi->i_gc_rwsem[WRITE]);
>  		err = f2fs_map_blocks(inode, &map, F2FS_GET_BLOCK_PRECACHE);
> -- 
> 2.49.0
> 
> Thanks,
> 
> >  			goto sync_out;
> >  		case F2FS_GET_BLOCK_BMAP:
> >  			map->m_pblk = 0;


We have already reset m_flags (map->m_flags = 0) in f2fs_map_blocks().

I think that this bug is caused by we missed to reset m_flags when we
goto next_dnode in below case：

Data layout is something like this:
dnode1:                     dnode2:
[0]      A                  [0]    NEW_ADDR
[1]      A+1                [1]    0x0
...
[1016]   A+1016
[1017]   B (B!=A+1017)      [1017] 0x0

we map the last block(valid blkaddr) in dnode1:
map->m_flags |= F2FS_MAP_MAPPED;
map->m_pblk = blkaddr(valid blkaddr);
map->m_len = 1;
then we goto next_dnode, meet the first block in dnode2(hole), goto sync_out:
map->m_flags & F2FS_MAP_MAPPED == true, and we make wrong blkaddr/len for extent_info.

Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents

[Chao Yu via Linux-f2fs-devel]

On 9/11/2025 5:07 PM, wangzijie wrote:
>> On 9/10/25 21:58, wangzijie wrote:
>>> When the data layout is like this:
>>> dnode1:                     dnode2:
>>> [0]      A                  [0]    NEW_ADDR
>>> [1]      A+1                [1]    0x0
>>> ...                         ....
>>> [1016]   A+1016
>>> [1017]   B (B!=A+1017)      [1017] 0x0
>>>
>>> We can build this kind of layout by following steps(with i_extra_isize:36):
>>> ./f2fs_io write 1 0 1881 rand dsync testfile
>>> ./f2fs_io write 1 1881 1 rand buffered testfile
>>> ./f2fs_io fallocate 0 7708672 4096 testfile
>>>
>>> And when we map first data block in dnode2, we will get wrong extent_info data:
>>> map->m_len = 1
>>> ofs = start_pgofs - map->m_lblk = 1882 - 1881 = 1
>>>
>>> ei.fofs = start_pgofs = 1882
>>> ei.len = map->m_len - ofs = 1 - 1 = 0
>>>
>>> Fix it by skipping updating this kind of extent info.
>>>
>>> Signed-off-by: wangzijie <wangzijie1@honor.com>
>>> ---
>>>   fs/f2fs/data.c | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>> index 7961e0ddf..b8bb71852 100644
>>> --- a/fs/f2fs/data.c
>>> +++ b/fs/f2fs/data.c
>>> @@ -1649,6 +1649,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>   
>>>   		switch (flag) {
>>>   		case F2FS_GET_BLOCK_PRECACHE:
>>> +			if (__is_valid_data_blkaddr(map->m_pblk) &&
>>> +				start_pgofs - map->m_lblk == map->m_len)
>>> +				map->m_flags &= ~F2FS_MAP_MAPPED;
>>
>> It looks we missed to reset value for map variable in f2fs_precache_extents(),
>> what do you think of this?
>>
>> ---
>>   fs/f2fs/file.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>> index 1aae4361d0a8..2b14151d4130 100644
>> --- a/fs/f2fs/file.c
>> +++ b/fs/f2fs/file.c
>> @@ -3599,7 +3599,7 @@ static int f2fs_ioc_io_prio(struct file *filp, unsigned long arg)
>>   int f2fs_precache_extents(struct inode *inode)
>>   {
>>   	struct f2fs_inode_info *fi = F2FS_I(inode);
>> -	struct f2fs_map_blocks map;
>> +	struct f2fs_map_blocks map = { 0 };
>>   	pgoff_t m_next_extent;
>>   	loff_t end;
>>   	int err;
>> @@ -3617,6 +3617,8 @@ int f2fs_precache_extents(struct inode *inode)
>>
>>   	while (map.m_lblk < end) {
>>   		map.m_len = end - map.m_lblk;
>> +		map.m_pblk = 0;
>> +		map.m_flags = 0;
>>
>>   		f2fs_down_write(&fi->i_gc_rwsem[WRITE]);
>>   		err = f2fs_map_blocks(inode, &map, F2FS_GET_BLOCK_PRECACHE);
>> -- 
>> 2.49.0
>>
>> Thanks,
>>
>>>   			goto sync_out;
>>>   		case F2FS_GET_BLOCK_BMAP:
>>>   			map->m_pblk = 0;
> 
> 
> We have already reset m_flags (map->m_flags = 0) in f2fs_map_blocks().

Zijie:

Oops, that's right, thanks for correcting me.

> 
> I think that this bug is caused by we missed to reset m_flags when we
> goto next_dnode in below case：
> 
> Data layout is something like this:
> dnode1:                     dnode2:
> [0]      A                  [0]    NEW_ADDR
> [1]      A+1                [1]    0x0
> ...
> [1016]   A+1016
> [1017]   B (B!=A+1017)      [1017] 0x0
> 
> we map the last block(valid blkaddr) in dnode1:
> map->m_flags |= F2FS_MAP_MAPPED;
> map->m_pblk = blkaddr(valid blkaddr);
> map->m_len = 1;
> then we goto next_dnode, meet the first block in dnode2(hole), goto sync_out:
> map->m_flags & F2FS_MAP_MAPPED == true, and we make wrong blkaddr/len for extent_info.

So, can you please add above explanation into commit message? that
should be helpful for understanding the problem more clearly.

Please take a look at this case w/ your patch:

mkfs.f2fs -O extra_attr,compression /dev/vdb -f
mount /dev/vdb /mnt/f2fs -o mode=lfs
cd /mnt/f2fs
f2fs_io write 1 0 1883 rand dsync testfile
f2fs_io fallocate 0 7712768 4096 testfile
f2fs_io write 1 1881 1 rand buffered testfile
xfs_io testfile -c "fsync"
cd /
umount /mnt/f2fs
mount /dev/vdb /mnt/f2fs
f2fs_io precache_extents /mnt/f2fs/testfile
umount /mnt/f2fs

          f2fs_io-733     [010] .....    78.134136: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 4, pgofs = 1882, len = 0, blkaddr = 17410, c_len = 0

I suspect we need this?

@@ -1784,7 +1781,8 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
         }

         if (flag == F2FS_GET_BLOCK_PRECACHE) {
-               if (map->m_flags & F2FS_MAP_MAPPED) {
+               if ((map->m_flags & F2FS_MAP_MAPPED) &&
+                       (map->m_len - ofs)) {
                         unsigned int ofs = start_pgofs - map->m_lblk;

                         f2fs_update_read_extent_cache_range(&dn,

BTW, I find another bug, if one blkaddr is adjcent to previous extent,
but and it is valid, we need to set m_next_extent to pgofs rather than
pgofs + 1.

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index cbf8841642c7..ac88ed68059c 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1789,8 +1789,11 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
                                 start_pgofs, map->m_pblk + ofs,
                                 map->m_len - ofs);
                 }
-               if (map->m_next_extent)
-                       *map->m_next_extent = pgofs + 1;
+               if (map->m_next_extent) {
+                       *map->m_next_extent = pgofs;
+                       if (!__is_valid_data_blkaddr(blkaddr))
+                               *map->m_next_extent += 1;
+               }
         }
         f2fs_put_dnode(&dn);


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

Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents

[Chao Yu]

On 9/11/2025 5:07 PM, wangzijie wrote:
>> On 9/10/25 21:58, wangzijie wrote:
>>> When the data layout is like this:
>>> dnode1:                     dnode2:
>>> [0]      A                  [0]    NEW_ADDR
>>> [1]      A+1                [1]    0x0
>>> ...                         ....
>>> [1016]   A+1016
>>> [1017]   B (B!=A+1017)      [1017] 0x0
>>>
>>> We can build this kind of layout by following steps(with i_extra_isize:36):
>>> ./f2fs_io write 1 0 1881 rand dsync testfile
>>> ./f2fs_io write 1 1881 1 rand buffered testfile
>>> ./f2fs_io fallocate 0 7708672 4096 testfile
>>>
>>> And when we map first data block in dnode2, we will get wrong extent_info data:
>>> map->m_len = 1
>>> ofs = start_pgofs - map->m_lblk = 1882 - 1881 = 1
>>>
>>> ei.fofs = start_pgofs = 1882
>>> ei.len = map->m_len - ofs = 1 - 1 = 0
>>>
>>> Fix it by skipping updating this kind of extent info.
>>>
>>> Signed-off-by: wangzijie <wangzijie1@honor.com>
>>> ---
>>>   fs/f2fs/data.c | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>> index 7961e0ddf..b8bb71852 100644
>>> --- a/fs/f2fs/data.c
>>> +++ b/fs/f2fs/data.c
>>> @@ -1649,6 +1649,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>   
>>>   		switch (flag) {
>>>   		case F2FS_GET_BLOCK_PRECACHE:
>>> +			if (__is_valid_data_blkaddr(map->m_pblk) &&
>>> +				start_pgofs - map->m_lblk == map->m_len)
>>> +				map->m_flags &= ~F2FS_MAP_MAPPED;
>>
>> It looks we missed to reset value for map variable in f2fs_precache_extents(),
>> what do you think of this?
>>
>> ---
>>   fs/f2fs/file.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>> index 1aae4361d0a8..2b14151d4130 100644
>> --- a/fs/f2fs/file.c
>> +++ b/fs/f2fs/file.c
>> @@ -3599,7 +3599,7 @@ static int f2fs_ioc_io_prio(struct file *filp, unsigned long arg)
>>   int f2fs_precache_extents(struct inode *inode)
>>   {
>>   	struct f2fs_inode_info *fi = F2FS_I(inode);
>> -	struct f2fs_map_blocks map;
>> +	struct f2fs_map_blocks map = { 0 };
>>   	pgoff_t m_next_extent;
>>   	loff_t end;
>>   	int err;
>> @@ -3617,6 +3617,8 @@ int f2fs_precache_extents(struct inode *inode)
>>
>>   	while (map.m_lblk < end) {
>>   		map.m_len = end - map.m_lblk;
>> +		map.m_pblk = 0;
>> +		map.m_flags = 0;
>>
>>   		f2fs_down_write(&fi->i_gc_rwsem[WRITE]);
>>   		err = f2fs_map_blocks(inode, &map, F2FS_GET_BLOCK_PRECACHE);
>> -- 
>> 2.49.0
>>
>> Thanks,
>>
>>>   			goto sync_out;
>>>   		case F2FS_GET_BLOCK_BMAP:
>>>   			map->m_pblk = 0;
> 
> 
> We have already reset m_flags (map->m_flags = 0) in f2fs_map_blocks().

Zijie:

Oops, that's right, thanks for correcting me.

> 
> I think that this bug is caused by we missed to reset m_flags when we
> goto next_dnode in below case：
> 
> Data layout is something like this:
> dnode1:                     dnode2:
> [0]      A                  [0]    NEW_ADDR
> [1]      A+1                [1]    0x0
> ...
> [1016]   A+1016
> [1017]   B (B!=A+1017)      [1017] 0x0
> 
> we map the last block(valid blkaddr) in dnode1:
> map->m_flags |= F2FS_MAP_MAPPED;
> map->m_pblk = blkaddr(valid blkaddr);
> map->m_len = 1;
> then we goto next_dnode, meet the first block in dnode2(hole), goto sync_out:
> map->m_flags & F2FS_MAP_MAPPED == true, and we make wrong blkaddr/len for extent_info.

So, can you please add above explanation into commit message? that
should be helpful for understanding the problem more clearly.

Please take a look at this case w/ your patch:

mkfs.f2fs -O extra_attr,compression /dev/vdb -f
mount /dev/vdb /mnt/f2fs -o mode=lfs
cd /mnt/f2fs
f2fs_io write 1 0 1883 rand dsync testfile
f2fs_io fallocate 0 7712768 4096 testfile
f2fs_io write 1 1881 1 rand buffered testfile
xfs_io testfile -c "fsync"
cd /
umount /mnt/f2fs
mount /dev/vdb /mnt/f2fs
f2fs_io precache_extents /mnt/f2fs/testfile
umount /mnt/f2fs

          f2fs_io-733     [010] .....    78.134136: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 4, pgofs = 1882, len = 0, blkaddr = 17410, c_len = 0

I suspect we need this?

@@ -1784,7 +1781,8 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
         }

         if (flag == F2FS_GET_BLOCK_PRECACHE) {
-               if (map->m_flags & F2FS_MAP_MAPPED) {
+               if ((map->m_flags & F2FS_MAP_MAPPED) &&
+                       (map->m_len - ofs)) {
                         unsigned int ofs = start_pgofs - map->m_lblk;

                         f2fs_update_read_extent_cache_range(&dn,

BTW, I find another bug, if one blkaddr is adjcent to previous extent,
but and it is valid, we need to set m_next_extent to pgofs rather than
pgofs + 1.

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index cbf8841642c7..ac88ed68059c 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1789,8 +1789,11 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
                                 start_pgofs, map->m_pblk + ofs,
                                 map->m_len - ofs);
                 }
-               if (map->m_next_extent)
-                       *map->m_next_extent = pgofs + 1;
+               if (map->m_next_extent) {
+                       *map->m_next_extent = pgofs;
+                       if (!__is_valid_data_blkaddr(blkaddr))
+                               *map->m_next_extent += 1;
+               }
         }
         f2fs_put_dnode(&dn);

Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents

[wangzijie]

>On 9/11/2025 5:07 PM, wangzijie wrote:
>>> On 9/10/25 21:58, wangzijie wrote:
>>>> When the data layout is like this:
>>>> dnode1:                     dnode2:
>>>> [0]      A                  [0]    NEW_ADDR
>>>> [1]      A+1                [1]    0x0
>>>> ...                         ....
>>>> [1016]   A+1016
>>>> [1017]   B (B!=A+1017)      [1017] 0x0
>>>>
>>>> We can build this kind of layout by following steps(with i_extra_isize:36):
>>>> ./f2fs_io write 1 0 1881 rand dsync testfile
>>>> ./f2fs_io write 1 1881 1 rand buffered testfile
>>>> ./f2fs_io fallocate 0 7708672 4096 testfile
>>>>
>>>> And when we map first data block in dnode2, we will get wrong extent_info data:
>>>> map->m_len = 1
>>>> ofs = start_pgofs - map->m_lblk = 1882 - 1881 = 1
>>>>
>>>> ei.fofs = start_pgofs = 1882
>>>> ei.len = map->m_len - ofs = 1 - 1 = 0
>>>>
>>>> Fix it by skipping updating this kind of extent info.
>>>>
>>>> Signed-off-by: wangzijie <wangzijie1@honor.com>
>>>> ---
>>>>   fs/f2fs/data.c | 3 +++
>>>>   1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>> index 7961e0ddf..b8bb71852 100644
>>>> --- a/fs/f2fs/data.c
>>>> +++ b/fs/f2fs/data.c
>>>> @@ -1649,6 +1649,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>>   
>>>>   		switch (flag) {
>>>>   		case F2FS_GET_BLOCK_PRECACHE:
>>>> +			if (__is_valid_data_blkaddr(map->m_pblk) &&
>>>> +				start_pgofs - map->m_lblk == map->m_len)
>>>> +				map->m_flags &= ~F2FS_MAP_MAPPED;
>>>
>>> It looks we missed to reset value for map variable in f2fs_precache_extents(),
>>> what do you think of this?
>>>
>>> ---
>>>   fs/f2fs/file.c | 4 +++-
>>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>> index 1aae4361d0a8..2b14151d4130 100644
>>> --- a/fs/f2fs/file.c
>>> +++ b/fs/f2fs/file.c
>>> @@ -3599,7 +3599,7 @@ static int f2fs_ioc_io_prio(struct file *filp, unsigned long arg)
>>>   int f2fs_precache_extents(struct inode *inode)
>>>   {
>>>   	struct f2fs_inode_info *fi = F2FS_I(inode);
>>> -	struct f2fs_map_blocks map;
>>> +	struct f2fs_map_blocks map = { 0 };
>>>   	pgoff_t m_next_extent;
>>>   	loff_t end;
>>>   	int err;
>>> @@ -3617,6 +3617,8 @@ int f2fs_precache_extents(struct inode *inode)
>>>
>>>   	while (map.m_lblk < end) {
>>>   		map.m_len = end - map.m_lblk;
>>> +		map.m_pblk = 0;
>>> +		map.m_flags = 0;
>>>
>>>   		f2fs_down_write(&fi->i_gc_rwsem[WRITE]);
>>>   		err = f2fs_map_blocks(inode, &map, F2FS_GET_BLOCK_PRECACHE);
>>> -- 
>>> 2.49.0
>>>
>>> Thanks,
>>>
>>>>   			goto sync_out;
>>>>   		case F2FS_GET_BLOCK_BMAP:
>>>>   			map->m_pblk = 0;
>> 
>> 
>> We have already reset m_flags (map->m_flags = 0) in f2fs_map_blocks().
>
>Zijie:
>
>Oops, that's right, thanks for correcting me.
>
>> 
>> I think that this bug is caused by we missed to reset m_flags when we
>> goto next_dnode in below case：
>> 
>> Data layout is something like this:
>> dnode1:                     dnode2:
>> [0]      A                  [0]    NEW_ADDR
>> [1]      A+1                [1]    0x0
>> ...
>> [1016]   A+1016
>> [1017]   B (B!=A+1017)      [1017] 0x0
>> 
>> we map the last block(valid blkaddr) in dnode1:
>> map->m_flags |= F2FS_MAP_MAPPED;
>> map->m_pblk = blkaddr(valid blkaddr);
>> map->m_len = 1;
>> then we goto next_dnode, meet the first block in dnode2(hole), goto sync_out:
>> map->m_flags & F2FS_MAP_MAPPED == true, and we make wrong blkaddr/len for extent_info.
>
>So, can you please add above explanation into commit message? that
>should be helpful for understanding the problem more clearly.
>
>Please take a look at this case w/ your patch:
>
>mkfs.f2fs -O extra_attr,compression /dev/vdb -f
>mount /dev/vdb /mnt/f2fs -o mode=lfs
>cd /mnt/f2fs
>f2fs_io write 1 0 1883 rand dsync testfile
>f2fs_io fallocate 0 7712768 4096 testfile
>f2fs_io write 1 1881 1 rand buffered testfile
>xfs_io testfile -c "fsync"
>cd /
>umount /mnt/f2fs
>mount /dev/vdb /mnt/f2fs
>f2fs_io precache_extents /mnt/f2fs/testfile
>umount /mnt/f2fs
>
>          f2fs_io-733     [010] .....    78.134136: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 4, pgofs = 1882, len = 0, blkaddr = 17410, c_len = 0
>
>I suspect we need this?
>
>@@ -1784,7 +1781,8 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>         }
>
>         if (flag == F2FS_GET_BLOCK_PRECACHE) {
>-               if (map->m_flags & F2FS_MAP_MAPPED) {
>+               if ((map->m_flags & F2FS_MAP_MAPPED) &&
>+                       (map->m_len - ofs)) {
>                         unsigned int ofs = start_pgofs - map->m_lblk;
>
>                         f2fs_update_read_extent_cache_range(&dn,

Thanks for pointing out this. Let me find a way to cover these cases and do more test.

>BTW, I find another bug, if one blkaddr is adjcent to previous extent,
>but and it is valid, we need to set m_next_extent to pgofs rather than
>pgofs + 1.
>
>diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>index cbf8841642c7..ac88ed68059c 100644
>--- a/fs/f2fs/data.c
>+++ b/fs/f2fs/data.c
>@@ -1789,8 +1789,11 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>                                 start_pgofs, map->m_pblk + ofs,
>                                 map->m_len - ofs);
>                 }
>-               if (map->m_next_extent)
>-                       *map->m_next_extent = pgofs + 1;
>+               if (map->m_next_extent) {
>+                       *map->m_next_extent = pgofs;
>+                       if (!__is_valid_data_blkaddr(blkaddr))
>+                               *map->m_next_extent += 1;
>+               }
>         }
>         f2fs_put_dnode(&dn);

Maybe it can be this?
if (map->m_next_extent)
	*map->m_next_extent = is_hole ? pgofs + 1 : pgofs;


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

Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents

[wangzijie]

>On 9/11/2025 5:07 PM, wangzijie wrote:
>>> On 9/10/25 21:58, wangzijie wrote:
>>>> When the data layout is like this:
>>>> dnode1:                     dnode2:
>>>> [0]      A                  [0]    NEW_ADDR
>>>> [1]      A+1                [1]    0x0
>>>> ...                         ....
>>>> [1016]   A+1016
>>>> [1017]   B (B!=A+1017)      [1017] 0x0
>>>>
>>>> We can build this kind of layout by following steps(with i_extra_isize:36):
>>>> ./f2fs_io write 1 0 1881 rand dsync testfile
>>>> ./f2fs_io write 1 1881 1 rand buffered testfile
>>>> ./f2fs_io fallocate 0 7708672 4096 testfile
>>>>
>>>> And when we map first data block in dnode2, we will get wrong extent_info data:
>>>> map->m_len = 1
>>>> ofs = start_pgofs - map->m_lblk = 1882 - 1881 = 1
>>>>
>>>> ei.fofs = start_pgofs = 1882
>>>> ei.len = map->m_len - ofs = 1 - 1 = 0
>>>>
>>>> Fix it by skipping updating this kind of extent info.
>>>>
>>>> Signed-off-by: wangzijie <wangzijie1@honor.com>
>>>> ---
>>>>   fs/f2fs/data.c | 3 +++
>>>>   1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>> index 7961e0ddf..b8bb71852 100644
>>>> --- a/fs/f2fs/data.c
>>>> +++ b/fs/f2fs/data.c
>>>> @@ -1649,6 +1649,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>>   
>>>>   		switch (flag) {
>>>>   		case F2FS_GET_BLOCK_PRECACHE:
>>>> +			if (__is_valid_data_blkaddr(map->m_pblk) &&
>>>> +				start_pgofs - map->m_lblk == map->m_len)
>>>> +				map->m_flags &= ~F2FS_MAP_MAPPED;
>>>
>>> It looks we missed to reset value for map variable in f2fs_precache_extents(),
>>> what do you think of this?
>>>
>>> ---
>>>   fs/f2fs/file.c | 4 +++-
>>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>> index 1aae4361d0a8..2b14151d4130 100644
>>> --- a/fs/f2fs/file.c
>>> +++ b/fs/f2fs/file.c
>>> @@ -3599,7 +3599,7 @@ static int f2fs_ioc_io_prio(struct file *filp, unsigned long arg)
>>>   int f2fs_precache_extents(struct inode *inode)
>>>   {
>>>   	struct f2fs_inode_info *fi = F2FS_I(inode);
>>> -	struct f2fs_map_blocks map;
>>> +	struct f2fs_map_blocks map = { 0 };
>>>   	pgoff_t m_next_extent;
>>>   	loff_t end;
>>>   	int err;
>>> @@ -3617,6 +3617,8 @@ int f2fs_precache_extents(struct inode *inode)
>>>
>>>   	while (map.m_lblk < end) {
>>>   		map.m_len = end - map.m_lblk;
>>> +		map.m_pblk = 0;
>>> +		map.m_flags = 0;
>>>
>>>   		f2fs_down_write(&fi->i_gc_rwsem[WRITE]);
>>>   		err = f2fs_map_blocks(inode, &map, F2FS_GET_BLOCK_PRECACHE);
>>> -- 
>>> 2.49.0
>>>
>>> Thanks,
>>>
>>>>   			goto sync_out;
>>>>   		case F2FS_GET_BLOCK_BMAP:
>>>>   			map->m_pblk = 0;
>> 
>> 
>> We have already reset m_flags (map->m_flags = 0) in f2fs_map_blocks().
>
>Zijie:
>
>Oops, that's right, thanks for correcting me.
>
>> 
>> I think that this bug is caused by we missed to reset m_flags when we
>> goto next_dnode in below case：
>> 
>> Data layout is something like this:
>> dnode1:                     dnode2:
>> [0]      A                  [0]    NEW_ADDR
>> [1]      A+1                [1]    0x0
>> ...
>> [1016]   A+1016
>> [1017]   B (B!=A+1017)      [1017] 0x0
>> 
>> we map the last block(valid blkaddr) in dnode1:
>> map->m_flags |= F2FS_MAP_MAPPED;
>> map->m_pblk = blkaddr(valid blkaddr);
>> map->m_len = 1;
>> then we goto next_dnode, meet the first block in dnode2(hole), goto sync_out:
>> map->m_flags & F2FS_MAP_MAPPED == true, and we make wrong blkaddr/len for extent_info.
>
>So, can you please add above explanation into commit message? that
>should be helpful for understanding the problem more clearly.
>
>Please take a look at this case w/ your patch:
>
>mkfs.f2fs -O extra_attr,compression /dev/vdb -f
>mount /dev/vdb /mnt/f2fs -o mode=lfs
>cd /mnt/f2fs
>f2fs_io write 1 0 1883 rand dsync testfile
>f2fs_io fallocate 0 7712768 4096 testfile
>f2fs_io write 1 1881 1 rand buffered testfile
>xfs_io testfile -c "fsync"
>cd /
>umount /mnt/f2fs
>mount /dev/vdb /mnt/f2fs
>f2fs_io precache_extents /mnt/f2fs/testfile
>umount /mnt/f2fs
>
>          f2fs_io-733     [010] .....    78.134136: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 4, pgofs = 1882, len = 0, blkaddr = 17410, c_len = 0
>
>I suspect we need this?
>
>@@ -1784,7 +1781,8 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>         }
>
>         if (flag == F2FS_GET_BLOCK_PRECACHE) {
>-               if (map->m_flags & F2FS_MAP_MAPPED) {
>+               if ((map->m_flags & F2FS_MAP_MAPPED) &&
>+                       (map->m_len - ofs)) {
>                         unsigned int ofs = start_pgofs - map->m_lblk;
>
>                         f2fs_update_read_extent_cache_range(&dn,

Thanks for pointing out this. Let me find a way to cover these cases and do more test.

>BTW, I find another bug, if one blkaddr is adjcent to previous extent,
>but and it is valid, we need to set m_next_extent to pgofs rather than
>pgofs + 1.
>
>diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>index cbf8841642c7..ac88ed68059c 100644
>--- a/fs/f2fs/data.c
>+++ b/fs/f2fs/data.c
>@@ -1789,8 +1789,11 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>                                 start_pgofs, map->m_pblk + ofs,
>                                 map->m_len - ofs);
>                 }
>-               if (map->m_next_extent)
>-                       *map->m_next_extent = pgofs + 1;
>+               if (map->m_next_extent) {
>+                       *map->m_next_extent = pgofs;
>+                       if (!__is_valid_data_blkaddr(blkaddr))
>+                               *map->m_next_extent += 1;
>+               }
>         }
>         f2fs_put_dnode(&dn);

Maybe it can be this?
if (map->m_next_extent)
	*map->m_next_extent = is_hole ? pgofs + 1 : pgofs;

Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents

[Chao Yu via Linux-f2fs-devel]

On 9/12/2025 11:36 AM, wangzijie wrote:
>> On 9/11/2025 5:07 PM, wangzijie wrote:
>>>> On 9/10/25 21:58, wangzijie wrote:
>>>>> When the data layout is like this:
>>>>> dnode1:                     dnode2:
>>>>> [0]      A                  [0]    NEW_ADDR
>>>>> [1]      A+1                [1]    0x0
>>>>> ...                         ....
>>>>> [1016]   A+1016
>>>>> [1017]   B (B!=A+1017)      [1017] 0x0
>>>>>
>>>>> We can build this kind of layout by following steps(with i_extra_isize:36):
>>>>> ./f2fs_io write 1 0 1881 rand dsync testfile
>>>>> ./f2fs_io write 1 1881 1 rand buffered testfile
>>>>> ./f2fs_io fallocate 0 7708672 4096 testfile
>>>>>
>>>>> And when we map first data block in dnode2, we will get wrong extent_info data:
>>>>> map->m_len = 1
>>>>> ofs = start_pgofs - map->m_lblk = 1882 - 1881 = 1
>>>>>
>>>>> ei.fofs = start_pgofs = 1882
>>>>> ei.len = map->m_len - ofs = 1 - 1 = 0
>>>>>
>>>>> Fix it by skipping updating this kind of extent info.
>>>>>
>>>>> Signed-off-by: wangzijie <wangzijie1@honor.com>
>>>>> ---
>>>>>    fs/f2fs/data.c | 3 +++
>>>>>    1 file changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>>> index 7961e0ddf..b8bb71852 100644
>>>>> --- a/fs/f2fs/data.c
>>>>> +++ b/fs/f2fs/data.c
>>>>> @@ -1649,6 +1649,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>>>    
>>>>>    		switch (flag) {
>>>>>    		case F2FS_GET_BLOCK_PRECACHE:
>>>>> +			if (__is_valid_data_blkaddr(map->m_pblk) &&
>>>>> +				start_pgofs - map->m_lblk == map->m_len)
>>>>> +				map->m_flags &= ~F2FS_MAP_MAPPED;
>>>>
>>>> It looks we missed to reset value for map variable in f2fs_precache_extents(),
>>>> what do you think of this?
>>>>
>>>> ---
>>>>    fs/f2fs/file.c | 4 +++-
>>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>>> index 1aae4361d0a8..2b14151d4130 100644
>>>> --- a/fs/f2fs/file.c
>>>> +++ b/fs/f2fs/file.c
>>>> @@ -3599,7 +3599,7 @@ static int f2fs_ioc_io_prio(struct file *filp, unsigned long arg)
>>>>    int f2fs_precache_extents(struct inode *inode)
>>>>    {
>>>>    	struct f2fs_inode_info *fi = F2FS_I(inode);
>>>> -	struct f2fs_map_blocks map;
>>>> +	struct f2fs_map_blocks map = { 0 };
>>>>    	pgoff_t m_next_extent;
>>>>    	loff_t end;
>>>>    	int err;
>>>> @@ -3617,6 +3617,8 @@ int f2fs_precache_extents(struct inode *inode)
>>>>
>>>>    	while (map.m_lblk < end) {
>>>>    		map.m_len = end - map.m_lblk;
>>>> +		map.m_pblk = 0;
>>>> +		map.m_flags = 0;
>>>>
>>>>    		f2fs_down_write(&fi->i_gc_rwsem[WRITE]);
>>>>    		err = f2fs_map_blocks(inode, &map, F2FS_GET_BLOCK_PRECACHE);
>>>> -- 
>>>> 2.49.0
>>>>
>>>> Thanks,
>>>>
>>>>>    			goto sync_out;
>>>>>    		case F2FS_GET_BLOCK_BMAP:
>>>>>    			map->m_pblk = 0;
>>>
>>>
>>> We have already reset m_flags (map->m_flags = 0) in f2fs_map_blocks().
>>
>> Zijie:
>>
>> Oops, that's right, thanks for correcting me.
>>
>>>
>>> I think that this bug is caused by we missed to reset m_flags when we
>>> goto next_dnode in below case：
>>>
>>> Data layout is something like this:
>>> dnode1:                     dnode2:
>>> [0]      A                  [0]    NEW_ADDR
>>> [1]      A+1                [1]    0x0
>>> ...
>>> [1016]   A+1016
>>> [1017]   B (B!=A+1017)      [1017] 0x0
>>>
>>> we map the last block(valid blkaddr) in dnode1:
>>> map->m_flags |= F2FS_MAP_MAPPED;
>>> map->m_pblk = blkaddr(valid blkaddr);
>>> map->m_len = 1;
>>> then we goto next_dnode, meet the first block in dnode2(hole), goto sync_out:
>>> map->m_flags & F2FS_MAP_MAPPED == true, and we make wrong blkaddr/len for extent_info.
>>
>> So, can you please add above explanation into commit message? that
>> should be helpful for understanding the problem more clearly.
>>
>> Please take a look at this case w/ your patch:
>>
>> mkfs.f2fs -O extra_attr,compression /dev/vdb -f
>> mount /dev/vdb /mnt/f2fs -o mode=lfs
>> cd /mnt/f2fs
>> f2fs_io write 1 0 1883 rand dsync testfile
>> f2fs_io fallocate 0 7712768 4096 testfile
>> f2fs_io write 1 1881 1 rand buffered testfile
>> xfs_io testfile -c "fsync"
>> cd /
>> umount /mnt/f2fs
>> mount /dev/vdb /mnt/f2fs
>> f2fs_io precache_extents /mnt/f2fs/testfile
>> umount /mnt/f2fs
>>
>>           f2fs_io-733     [010] .....    78.134136: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 4, pgofs = 1882, len = 0, blkaddr = 17410, c_len = 0
>>
>> I suspect we need this?
>>
>> @@ -1784,7 +1781,8 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>          }
>>
>>          if (flag == F2FS_GET_BLOCK_PRECACHE) {
>> -               if (map->m_flags & F2FS_MAP_MAPPED) {
>> +               if ((map->m_flags & F2FS_MAP_MAPPED) &&
>> +                       (map->m_len - ofs)) {
>>                          unsigned int ofs = start_pgofs - map->m_lblk;
>>
>>                          f2fs_update_read_extent_cache_range(&dn,
> 
> Thanks for pointing out this. Let me find a way to cover these cases and do more test.
> 
>> BTW, I find another bug, if one blkaddr is adjcent to previous extent,
>> but and it is valid, we need to set m_next_extent to pgofs rather than
>> pgofs + 1.
>>
>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>> index cbf8841642c7..ac88ed68059c 100644
>> --- a/fs/f2fs/data.c
>> +++ b/fs/f2fs/data.c
>> @@ -1789,8 +1789,11 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>                                  start_pgofs, map->m_pblk + ofs,
>>                                  map->m_len - ofs);
>>                  }
>> -               if (map->m_next_extent)
>> -                       *map->m_next_extent = pgofs + 1;
>> +               if (map->m_next_extent) {
>> +                       *map->m_next_extent = pgofs;
>> +                       if (!__is_valid_data_blkaddr(blkaddr))
>> +                               *map->m_next_extent += 1;
>> +               }
>>          }
>>          f2fs_put_dnode(&dn);
> 
> Maybe it can be this?
> if (map->m_next_extent)
> 	*map->m_next_extent = is_hole ? pgofs + 1 : pgofs;

It's better, will update, thank you. :)

Thanks,




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

Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents

[Chao Yu]

On 9/12/2025 11:36 AM, wangzijie wrote:
>> On 9/11/2025 5:07 PM, wangzijie wrote:
>>>> On 9/10/25 21:58, wangzijie wrote:
>>>>> When the data layout is like this:
>>>>> dnode1:                     dnode2:
>>>>> [0]      A                  [0]    NEW_ADDR
>>>>> [1]      A+1                [1]    0x0
>>>>> ...                         ....
>>>>> [1016]   A+1016
>>>>> [1017]   B (B!=A+1017)      [1017] 0x0
>>>>>
>>>>> We can build this kind of layout by following steps(with i_extra_isize:36):
>>>>> ./f2fs_io write 1 0 1881 rand dsync testfile
>>>>> ./f2fs_io write 1 1881 1 rand buffered testfile
>>>>> ./f2fs_io fallocate 0 7708672 4096 testfile
>>>>>
>>>>> And when we map first data block in dnode2, we will get wrong extent_info data:
>>>>> map->m_len = 1
>>>>> ofs = start_pgofs - map->m_lblk = 1882 - 1881 = 1
>>>>>
>>>>> ei.fofs = start_pgofs = 1882
>>>>> ei.len = map->m_len - ofs = 1 - 1 = 0
>>>>>
>>>>> Fix it by skipping updating this kind of extent info.
>>>>>
>>>>> Signed-off-by: wangzijie <wangzijie1@honor.com>
>>>>> ---
>>>>>    fs/f2fs/data.c | 3 +++
>>>>>    1 file changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>>> index 7961e0ddf..b8bb71852 100644
>>>>> --- a/fs/f2fs/data.c
>>>>> +++ b/fs/f2fs/data.c
>>>>> @@ -1649,6 +1649,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>>>    
>>>>>    		switch (flag) {
>>>>>    		case F2FS_GET_BLOCK_PRECACHE:
>>>>> +			if (__is_valid_data_blkaddr(map->m_pblk) &&
>>>>> +				start_pgofs - map->m_lblk == map->m_len)
>>>>> +				map->m_flags &= ~F2FS_MAP_MAPPED;
>>>>
>>>> It looks we missed to reset value for map variable in f2fs_precache_extents(),
>>>> what do you think of this?
>>>>
>>>> ---
>>>>    fs/f2fs/file.c | 4 +++-
>>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>>> index 1aae4361d0a8..2b14151d4130 100644
>>>> --- a/fs/f2fs/file.c
>>>> +++ b/fs/f2fs/file.c
>>>> @@ -3599,7 +3599,7 @@ static int f2fs_ioc_io_prio(struct file *filp, unsigned long arg)
>>>>    int f2fs_precache_extents(struct inode *inode)
>>>>    {
>>>>    	struct f2fs_inode_info *fi = F2FS_I(inode);
>>>> -	struct f2fs_map_blocks map;
>>>> +	struct f2fs_map_blocks map = { 0 };
>>>>    	pgoff_t m_next_extent;
>>>>    	loff_t end;
>>>>    	int err;
>>>> @@ -3617,6 +3617,8 @@ int f2fs_precache_extents(struct inode *inode)
>>>>
>>>>    	while (map.m_lblk < end) {
>>>>    		map.m_len = end - map.m_lblk;
>>>> +		map.m_pblk = 0;
>>>> +		map.m_flags = 0;
>>>>
>>>>    		f2fs_down_write(&fi->i_gc_rwsem[WRITE]);
>>>>    		err = f2fs_map_blocks(inode, &map, F2FS_GET_BLOCK_PRECACHE);
>>>> -- 
>>>> 2.49.0
>>>>
>>>> Thanks,
>>>>
>>>>>    			goto sync_out;
>>>>>    		case F2FS_GET_BLOCK_BMAP:
>>>>>    			map->m_pblk = 0;
>>>
>>>
>>> We have already reset m_flags (map->m_flags = 0) in f2fs_map_blocks().
>>
>> Zijie:
>>
>> Oops, that's right, thanks for correcting me.
>>
>>>
>>> I think that this bug is caused by we missed to reset m_flags when we
>>> goto next_dnode in below case：
>>>
>>> Data layout is something like this:
>>> dnode1:                     dnode2:
>>> [0]      A                  [0]    NEW_ADDR
>>> [1]      A+1                [1]    0x0
>>> ...
>>> [1016]   A+1016
>>> [1017]   B (B!=A+1017)      [1017] 0x0
>>>
>>> we map the last block(valid blkaddr) in dnode1:
>>> map->m_flags |= F2FS_MAP_MAPPED;
>>> map->m_pblk = blkaddr(valid blkaddr);
>>> map->m_len = 1;
>>> then we goto next_dnode, meet the first block in dnode2(hole), goto sync_out:
>>> map->m_flags & F2FS_MAP_MAPPED == true, and we make wrong blkaddr/len for extent_info.
>>
>> So, can you please add above explanation into commit message? that
>> should be helpful for understanding the problem more clearly.
>>
>> Please take a look at this case w/ your patch:
>>
>> mkfs.f2fs -O extra_attr,compression /dev/vdb -f
>> mount /dev/vdb /mnt/f2fs -o mode=lfs
>> cd /mnt/f2fs
>> f2fs_io write 1 0 1883 rand dsync testfile
>> f2fs_io fallocate 0 7712768 4096 testfile
>> f2fs_io write 1 1881 1 rand buffered testfile
>> xfs_io testfile -c "fsync"
>> cd /
>> umount /mnt/f2fs
>> mount /dev/vdb /mnt/f2fs
>> f2fs_io precache_extents /mnt/f2fs/testfile
>> umount /mnt/f2fs
>>
>>           f2fs_io-733     [010] .....    78.134136: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 4, pgofs = 1882, len = 0, blkaddr = 17410, c_len = 0
>>
>> I suspect we need this?
>>
>> @@ -1784,7 +1781,8 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>          }
>>
>>          if (flag == F2FS_GET_BLOCK_PRECACHE) {
>> -               if (map->m_flags & F2FS_MAP_MAPPED) {
>> +               if ((map->m_flags & F2FS_MAP_MAPPED) &&
>> +                       (map->m_len - ofs)) {
>>                          unsigned int ofs = start_pgofs - map->m_lblk;
>>
>>                          f2fs_update_read_extent_cache_range(&dn,
> 
> Thanks for pointing out this. Let me find a way to cover these cases and do more test.
> 
>> BTW, I find another bug, if one blkaddr is adjcent to previous extent,
>> but and it is valid, we need to set m_next_extent to pgofs rather than
>> pgofs + 1.
>>
>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>> index cbf8841642c7..ac88ed68059c 100644
>> --- a/fs/f2fs/data.c
>> +++ b/fs/f2fs/data.c
>> @@ -1789,8 +1789,11 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>                                  start_pgofs, map->m_pblk + ofs,
>>                                  map->m_len - ofs);
>>                  }
>> -               if (map->m_next_extent)
>> -                       *map->m_next_extent = pgofs + 1;
>> +               if (map->m_next_extent) {
>> +                       *map->m_next_extent = pgofs;
>> +                       if (!__is_valid_data_blkaddr(blkaddr))
>> +                               *map->m_next_extent += 1;
>> +               }
>>          }
>>          f2fs_put_dnode(&dn);
> 
> Maybe it can be this?
> if (map->m_next_extent)
> 	*map->m_next_extent = is_hole ? pgofs + 1 : pgofs;

It's better, will update, thank you. :)

Thanks,

Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents

[wangzijie]

>On 9/12/2025 11:36 AM, wangzijie wrote:
>>> On 9/11/2025 5:07 PM, wangzijie wrote:
>>>>> On 9/10/25 21:58, wangzijie wrote:
>>>>>> When the data layout is like this:
>>>>>> dnode1:                     dnode2:
>>>>>> [0]      A                  [0]    NEW_ADDR
>>>>>> [1]      A+1                [1]    0x0
>>>>>> ...                         ....
>>>>>> [1016]   A+1016
>>>>>> [1017]   B (B!=A+1017)      [1017] 0x0
>>>>>>
>>>>>> We can build this kind of layout by following steps(with i_extra_isize:36):
>>>>>> ./f2fs_io write 1 0 1881 rand dsync testfile
>>>>>> ./f2fs_io write 1 1881 1 rand buffered testfile
>>>>>> ./f2fs_io fallocate 0 7708672 4096 testfile
>>>>>>
>>>>>> And when we map first data block in dnode2, we will get wrong extent_info data:
>>>>>> map->m_len = 1
>>>>>> ofs = start_pgofs - map->m_lblk = 1882 - 1881 = 1
>>>>>>
>>>>>> ei.fofs = start_pgofs = 1882
>>>>>> ei.len = map->m_len - ofs = 1 - 1 = 0
>>>>>>
>>>>>> Fix it by skipping updating this kind of extent info.
>>>>>>
>>>>>> Signed-off-by: wangzijie <wangzijie1@honor.com>
>>>>>> ---
>>>>>>    fs/f2fs/data.c | 3 +++
>>>>>>    1 file changed, 3 insertions(+)
>>>>>>
>>>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>>>> index 7961e0ddf..b8bb71852 100644
>>>>>> --- a/fs/f2fs/data.c
>>>>>> +++ b/fs/f2fs/data.c
>>>>>> @@ -1649,6 +1649,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>>>>    
>>>>>>    		switch (flag) {
>>>>>>    		case F2FS_GET_BLOCK_PRECACHE:
>>>>>> +			if (__is_valid_data_blkaddr(map->m_pblk) &&
>>>>>> +				start_pgofs - map->m_lblk == map->m_len)
>>>>>> +				map->m_flags &= ~F2FS_MAP_MAPPED;
>>>>>
>>>>> It looks we missed to reset value for map variable in f2fs_precache_extents(),
>>>>> what do you think of this?
>>>>>
>>>>> ---
>>>>>    fs/f2fs/file.c | 4 +++-
>>>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>>>> index 1aae4361d0a8..2b14151d4130 100644
>>>>> --- a/fs/f2fs/file.c
>>>>> +++ b/fs/f2fs/file.c
>>>>> @@ -3599,7 +3599,7 @@ static int f2fs_ioc_io_prio(struct file *filp, unsigned long arg)
>>>>>    int f2fs_precache_extents(struct inode *inode)
>>>>>    {
>>>>>    	struct f2fs_inode_info *fi = F2FS_I(inode);
>>>>> -	struct f2fs_map_blocks map;
>>>>> +	struct f2fs_map_blocks map = { 0 };
>>>>>    	pgoff_t m_next_extent;
>>>>>    	loff_t end;
>>>>>    	int err;
>>>>> @@ -3617,6 +3617,8 @@ int f2fs_precache_extents(struct inode *inode)
>>>>>
>>>>>    	while (map.m_lblk < end) {
>>>>>    		map.m_len = end - map.m_lblk;
>>>>> +		map.m_pblk = 0;
>>>>> +		map.m_flags = 0;
>>>>>
>>>>>    		f2fs_down_write(&fi->i_gc_rwsem[WRITE]);
>>>>>    		err = f2fs_map_blocks(inode, &map, F2FS_GET_BLOCK_PRECACHE);
>>>>> -- 
>>>>> 2.49.0
>>>>>
>>>>> Thanks,
>>>>>
>>>>>>    			goto sync_out;
>>>>>>    		case F2FS_GET_BLOCK_BMAP:
>>>>>>    			map->m_pblk = 0;
>>>>
>>>>
>>>> We have already reset m_flags (map->m_flags = 0) in f2fs_map_blocks().
>>>
>>> Zijie:
>>>
>>> Oops, that's right, thanks for correcting me.
>>>
>>>>
>>>> I think that this bug is caused by we missed to reset m_flags when we
>>>> goto next_dnode in below case：
>>>>
>>>> Data layout is something like this:
>>>> dnode1:                     dnode2:
>>>> [0]      A                  [0]    NEW_ADDR
>>>> [1]      A+1                [1]    0x0
>>>> ...
>>>> [1016]   A+1016
>>>> [1017]   B (B!=A+1017)      [1017] 0x0
>>>>
>>>> we map the last block(valid blkaddr) in dnode1:
>>>> map->m_flags |= F2FS_MAP_MAPPED;
>>>> map->m_pblk = blkaddr(valid blkaddr);
>>>> map->m_len = 1;
>>>> then we goto next_dnode, meet the first block in dnode2(hole), goto sync_out:
>>>> map->m_flags & F2FS_MAP_MAPPED == true, and we make wrong blkaddr/len for extent_info.
>>>
>>> So, can you please add above explanation into commit message? that
>>> should be helpful for understanding the problem more clearly.
>>>
>>> Please take a look at this case w/ your patch:
>>>
>>> mkfs.f2fs -O extra_attr,compression /dev/vdb -f
>>> mount /dev/vdb /mnt/f2fs -o mode=lfs
>>> cd /mnt/f2fs
>>> f2fs_io write 1 0 1883 rand dsync testfile
>>> f2fs_io fallocate 0 7712768 4096 testfile
>>> f2fs_io write 1 1881 1 rand buffered testfile
>>> xfs_io testfile -c "fsync"
>>> cd /
>>> umount /mnt/f2fs
>>> mount /dev/vdb /mnt/f2fs
>>> f2fs_io precache_extents /mnt/f2fs/testfile
>>> umount /mnt/f2fs
>>>
>>>           f2fs_io-733     [010] .....    78.134136: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 4, pgofs = 1882, len = 0, blkaddr = 17410, c_len = 0
>>>
>>> I suspect we need this?
>>>
>>> @@ -1784,7 +1781,8 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>          }
>>>
>>>          if (flag == F2FS_GET_BLOCK_PRECACHE) {
>>> -               if (map->m_flags & F2FS_MAP_MAPPED) {
>>> +               if ((map->m_flags & F2FS_MAP_MAPPED) &&
>>> +                       (map->m_len - ofs)) {
>>>                          unsigned int ofs = start_pgofs - map->m_lblk;
>>>
>>>                          f2fs_update_read_extent_cache_range(&dn,
>> 
>> Thanks for pointing out this. Let me find a way to cover these cases and do more test.
>> 
>>> BTW, I find another bug, if one blkaddr is adjcent to previous extent,
>>> but and it is valid, we need to set m_next_extent to pgofs rather than
>>> pgofs + 1.
>>>
>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>> index cbf8841642c7..ac88ed68059c 100644
>>> --- a/fs/f2fs/data.c
>>> +++ b/fs/f2fs/data.c
>>> @@ -1789,8 +1789,11 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>                                  start_pgofs, map->m_pblk + ofs,
>>>                                  map->m_len - ofs);
>>>                  }
>>> -               if (map->m_next_extent)
>>> -                       *map->m_next_extent = pgofs + 1;
>>> +               if (map->m_next_extent) {
>>> +                       *map->m_next_extent = pgofs;
>>> +                       if (!__is_valid_data_blkaddr(blkaddr))
>>> +                               *map->m_next_extent += 1;
>>> +               }
>>>          }
>>>          f2fs_put_dnode(&dn);
>> 
>> Maybe it can be this?
>> if (map->m_next_extent)
>> 	*map->m_next_extent = is_hole ? pgofs + 1 : pgofs;
>
>It's better, will update, thank you. :)
>
>Thanks,

Hi Chao,
I test some cases with this change:

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 7961e0ddf..7093fdc95 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1777,13 +1777,13 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
        if (flag == F2FS_GET_BLOCK_PRECACHE) {
                if (map->m_flags & F2FS_MAP_MAPPED) {
                        unsigned int ofs = start_pgofs - map->m_lblk;
-
-                       f2fs_update_read_extent_cache_range(&dn,
-                               start_pgofs, map->m_pblk + ofs,
-                               map->m_len - ofs);
+                       if (map->m_len - ofs > 0)
+                               f2fs_update_read_extent_cache_range(&dn,
+                                       start_pgofs, map->m_pblk + ofs,
+                                       map->m_len - ofs);
                }
                if (map->m_next_extent)
-                       *map->m_next_extent = pgofs + 1;
+                       *map->m_next_extent = is_hole ? pgofs + 1 : pgofs;
        }
        f2fs_put_dnode(&dn);
 unlock_out:


test cases:

case1:
dnode1:                     dnode2:
[0]      A                  [0]    NEW_ADDR
[1]      A+1                [1]    0x0
...                         ....
[1016]   A+1016
[1017]   B (B!=A+1017)      [1017] 0x0

case2:
dnode1:                     dnode2:
[0]      A                  [0]    C (C!=B+1)
[1]      A+1                [1]    C+1
...                         ....
[1016]   A+1016
[1017]   B (B!=A+1017)      [1017] 0x0

case3:
dnode1:                     dnode2:
[0]      A                  [0]    C (C!=B+2)
[1]      A+1                [1]    C+1
...                         ....
[1015]   A+1015
[1016]   B (B!=A+1016)
[1017]   B+1                [1017] 0x0

case4:
one blkaddr is adjcent to previous extent, and it is valid.

And from the result, it seems this change can cover these
situations correctly.
Do we need a patch with this change?


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

Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents

[wangzijie]

>On 9/12/2025 11:36 AM, wangzijie wrote:
>>> On 9/11/2025 5:07 PM, wangzijie wrote:
>>>>> On 9/10/25 21:58, wangzijie wrote:
>>>>>> When the data layout is like this:
>>>>>> dnode1:                     dnode2:
>>>>>> [0]      A                  [0]    NEW_ADDR
>>>>>> [1]      A+1                [1]    0x0
>>>>>> ...                         ....
>>>>>> [1016]   A+1016
>>>>>> [1017]   B (B!=A+1017)      [1017] 0x0
>>>>>>
>>>>>> We can build this kind of layout by following steps(with i_extra_isize:36):
>>>>>> ./f2fs_io write 1 0 1881 rand dsync testfile
>>>>>> ./f2fs_io write 1 1881 1 rand buffered testfile
>>>>>> ./f2fs_io fallocate 0 7708672 4096 testfile
>>>>>>
>>>>>> And when we map first data block in dnode2, we will get wrong extent_info data:
>>>>>> map->m_len = 1
>>>>>> ofs = start_pgofs - map->m_lblk = 1882 - 1881 = 1
>>>>>>
>>>>>> ei.fofs = start_pgofs = 1882
>>>>>> ei.len = map->m_len - ofs = 1 - 1 = 0
>>>>>>
>>>>>> Fix it by skipping updating this kind of extent info.
>>>>>>
>>>>>> Signed-off-by: wangzijie <wangzijie1@honor.com>
>>>>>> ---
>>>>>>    fs/f2fs/data.c | 3 +++
>>>>>>    1 file changed, 3 insertions(+)
>>>>>>
>>>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>>>> index 7961e0ddf..b8bb71852 100644
>>>>>> --- a/fs/f2fs/data.c
>>>>>> +++ b/fs/f2fs/data.c
>>>>>> @@ -1649,6 +1649,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>>>>    
>>>>>>    		switch (flag) {
>>>>>>    		case F2FS_GET_BLOCK_PRECACHE:
>>>>>> +			if (__is_valid_data_blkaddr(map->m_pblk) &&
>>>>>> +				start_pgofs - map->m_lblk == map->m_len)
>>>>>> +				map->m_flags &= ~F2FS_MAP_MAPPED;
>>>>>
>>>>> It looks we missed to reset value for map variable in f2fs_precache_extents(),
>>>>> what do you think of this?
>>>>>
>>>>> ---
>>>>>    fs/f2fs/file.c | 4 +++-
>>>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>>>> index 1aae4361d0a8..2b14151d4130 100644
>>>>> --- a/fs/f2fs/file.c
>>>>> +++ b/fs/f2fs/file.c
>>>>> @@ -3599,7 +3599,7 @@ static int f2fs_ioc_io_prio(struct file *filp, unsigned long arg)
>>>>>    int f2fs_precache_extents(struct inode *inode)
>>>>>    {
>>>>>    	struct f2fs_inode_info *fi = F2FS_I(inode);
>>>>> -	struct f2fs_map_blocks map;
>>>>> +	struct f2fs_map_blocks map = { 0 };
>>>>>    	pgoff_t m_next_extent;
>>>>>    	loff_t end;
>>>>>    	int err;
>>>>> @@ -3617,6 +3617,8 @@ int f2fs_precache_extents(struct inode *inode)
>>>>>
>>>>>    	while (map.m_lblk < end) {
>>>>>    		map.m_len = end - map.m_lblk;
>>>>> +		map.m_pblk = 0;
>>>>> +		map.m_flags = 0;
>>>>>
>>>>>    		f2fs_down_write(&fi->i_gc_rwsem[WRITE]);
>>>>>    		err = f2fs_map_blocks(inode, &map, F2FS_GET_BLOCK_PRECACHE);
>>>>> -- 
>>>>> 2.49.0
>>>>>
>>>>> Thanks,
>>>>>
>>>>>>    			goto sync_out;
>>>>>>    		case F2FS_GET_BLOCK_BMAP:
>>>>>>    			map->m_pblk = 0;
>>>>
>>>>
>>>> We have already reset m_flags (map->m_flags = 0) in f2fs_map_blocks().
>>>
>>> Zijie:
>>>
>>> Oops, that's right, thanks for correcting me.
>>>
>>>>
>>>> I think that this bug is caused by we missed to reset m_flags when we
>>>> goto next_dnode in below case：
>>>>
>>>> Data layout is something like this:
>>>> dnode1:                     dnode2:
>>>> [0]      A                  [0]    NEW_ADDR
>>>> [1]      A+1                [1]    0x0
>>>> ...
>>>> [1016]   A+1016
>>>> [1017]   B (B!=A+1017)      [1017] 0x0
>>>>
>>>> we map the last block(valid blkaddr) in dnode1:
>>>> map->m_flags |= F2FS_MAP_MAPPED;
>>>> map->m_pblk = blkaddr(valid blkaddr);
>>>> map->m_len = 1;
>>>> then we goto next_dnode, meet the first block in dnode2(hole), goto sync_out:
>>>> map->m_flags & F2FS_MAP_MAPPED == true, and we make wrong blkaddr/len for extent_info.
>>>
>>> So, can you please add above explanation into commit message? that
>>> should be helpful for understanding the problem more clearly.
>>>
>>> Please take a look at this case w/ your patch:
>>>
>>> mkfs.f2fs -O extra_attr,compression /dev/vdb -f
>>> mount /dev/vdb /mnt/f2fs -o mode=lfs
>>> cd /mnt/f2fs
>>> f2fs_io write 1 0 1883 rand dsync testfile
>>> f2fs_io fallocate 0 7712768 4096 testfile
>>> f2fs_io write 1 1881 1 rand buffered testfile
>>> xfs_io testfile -c "fsync"
>>> cd /
>>> umount /mnt/f2fs
>>> mount /dev/vdb /mnt/f2fs
>>> f2fs_io precache_extents /mnt/f2fs/testfile
>>> umount /mnt/f2fs
>>>
>>>           f2fs_io-733     [010] .....    78.134136: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 4, pgofs = 1882, len = 0, blkaddr = 17410, c_len = 0
>>>
>>> I suspect we need this?
>>>
>>> @@ -1784,7 +1781,8 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>          }
>>>
>>>          if (flag == F2FS_GET_BLOCK_PRECACHE) {
>>> -               if (map->m_flags & F2FS_MAP_MAPPED) {
>>> +               if ((map->m_flags & F2FS_MAP_MAPPED) &&
>>> +                       (map->m_len - ofs)) {
>>>                          unsigned int ofs = start_pgofs - map->m_lblk;
>>>
>>>                          f2fs_update_read_extent_cache_range(&dn,
>> 
>> Thanks for pointing out this. Let me find a way to cover these cases and do more test.
>> 
>>> BTW, I find another bug, if one blkaddr is adjcent to previous extent,
>>> but and it is valid, we need to set m_next_extent to pgofs rather than
>>> pgofs + 1.
>>>
>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>> index cbf8841642c7..ac88ed68059c 100644
>>> --- a/fs/f2fs/data.c
>>> +++ b/fs/f2fs/data.c
>>> @@ -1789,8 +1789,11 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>                                  start_pgofs, map->m_pblk + ofs,
>>>                                  map->m_len - ofs);
>>>                  }
>>> -               if (map->m_next_extent)
>>> -                       *map->m_next_extent = pgofs + 1;
>>> +               if (map->m_next_extent) {
>>> +                       *map->m_next_extent = pgofs;
>>> +                       if (!__is_valid_data_blkaddr(blkaddr))
>>> +                               *map->m_next_extent += 1;
>>> +               }
>>>          }
>>>          f2fs_put_dnode(&dn);
>> 
>> Maybe it can be this?
>> if (map->m_next_extent)
>> 	*map->m_next_extent = is_hole ? pgofs + 1 : pgofs;
>
>It's better, will update, thank you. :)
>
>Thanks,

Hi Chao,
I test some cases with this change:

diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
index 7961e0ddf..7093fdc95 100644
--- a/fs/f2fs/data.c
+++ b/fs/f2fs/data.c
@@ -1777,13 +1777,13 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
        if (flag == F2FS_GET_BLOCK_PRECACHE) {
                if (map->m_flags & F2FS_MAP_MAPPED) {
                        unsigned int ofs = start_pgofs - map->m_lblk;
-
-                       f2fs_update_read_extent_cache_range(&dn,
-                               start_pgofs, map->m_pblk + ofs,
-                               map->m_len - ofs);
+                       if (map->m_len - ofs > 0)
+                               f2fs_update_read_extent_cache_range(&dn,
+                                       start_pgofs, map->m_pblk + ofs,
+                                       map->m_len - ofs);
                }
                if (map->m_next_extent)
-                       *map->m_next_extent = pgofs + 1;
+                       *map->m_next_extent = is_hole ? pgofs + 1 : pgofs;
        }
        f2fs_put_dnode(&dn);
 unlock_out:


test cases:

case1:
dnode1:                     dnode2:
[0]      A                  [0]    NEW_ADDR
[1]      A+1                [1]    0x0
...                         ....
[1016]   A+1016
[1017]   B (B!=A+1017)      [1017] 0x0

case2:
dnode1:                     dnode2:
[0]      A                  [0]    C (C!=B+1)
[1]      A+1                [1]    C+1
...                         ....
[1016]   A+1016
[1017]   B (B!=A+1017)      [1017] 0x0

case3:
dnode1:                     dnode2:
[0]      A                  [0]    C (C!=B+2)
[1]      A+1                [1]    C+1
...                         ....
[1015]   A+1015
[1016]   B (B!=A+1016)
[1017]   B+1                [1017] 0x0

case4:
one blkaddr is adjcent to previous extent, and it is valid.

And from the result, it seems this change can cover these
situations correctly.
Do we need a patch with this change?

Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents

[Chao Yu via Linux-f2fs-devel]

On 9/12/2025 6:06 PM, wangzijie wrote:
>> On 9/12/2025 11:36 AM, wangzijie wrote:
>>>> On 9/11/2025 5:07 PM, wangzijie wrote:
>>>>>> On 9/10/25 21:58, wangzijie wrote:
>>>>>>> When the data layout is like this:
>>>>>>> dnode1:                     dnode2:
>>>>>>> [0]      A                  [0]    NEW_ADDR
>>>>>>> [1]      A+1                [1]    0x0
>>>>>>> ...                         ....
>>>>>>> [1016]   A+1016
>>>>>>> [1017]   B (B!=A+1017)      [1017] 0x0
>>>>>>>
>>>>>>> We can build this kind of layout by following steps(with i_extra_isize:36):
>>>>>>> ./f2fs_io write 1 0 1881 rand dsync testfile
>>>>>>> ./f2fs_io write 1 1881 1 rand buffered testfile
>>>>>>> ./f2fs_io fallocate 0 7708672 4096 testfile
>>>>>>>
>>>>>>> And when we map first data block in dnode2, we will get wrong extent_info data:
>>>>>>> map->m_len = 1
>>>>>>> ofs = start_pgofs - map->m_lblk = 1882 - 1881 = 1
>>>>>>>
>>>>>>> ei.fofs = start_pgofs = 1882
>>>>>>> ei.len = map->m_len - ofs = 1 - 1 = 0
>>>>>>>
>>>>>>> Fix it by skipping updating this kind of extent info.
>>>>>>>
>>>>>>> Signed-off-by: wangzijie <wangzijie1@honor.com>
>>>>>>> ---
>>>>>>>     fs/f2fs/data.c | 3 +++
>>>>>>>     1 file changed, 3 insertions(+)
>>>>>>>
>>>>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>>>>> index 7961e0ddf..b8bb71852 100644
>>>>>>> --- a/fs/f2fs/data.c
>>>>>>> +++ b/fs/f2fs/data.c
>>>>>>> @@ -1649,6 +1649,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>>>>>     
>>>>>>>     		switch (flag) {
>>>>>>>     		case F2FS_GET_BLOCK_PRECACHE:
>>>>>>> +			if (__is_valid_data_blkaddr(map->m_pblk) &&
>>>>>>> +				start_pgofs - map->m_lblk == map->m_len)
>>>>>>> +				map->m_flags &= ~F2FS_MAP_MAPPED;
>>>>>>
>>>>>> It looks we missed to reset value for map variable in f2fs_precache_extents(),
>>>>>> what do you think of this?
>>>>>>
>>>>>> ---
>>>>>>     fs/f2fs/file.c | 4 +++-
>>>>>>     1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>>>>> index 1aae4361d0a8..2b14151d4130 100644
>>>>>> --- a/fs/f2fs/file.c
>>>>>> +++ b/fs/f2fs/file.c
>>>>>> @@ -3599,7 +3599,7 @@ static int f2fs_ioc_io_prio(struct file *filp, unsigned long arg)
>>>>>>     int f2fs_precache_extents(struct inode *inode)
>>>>>>     {
>>>>>>     	struct f2fs_inode_info *fi = F2FS_I(inode);
>>>>>> -	struct f2fs_map_blocks map;
>>>>>> +	struct f2fs_map_blocks map = { 0 };
>>>>>>     	pgoff_t m_next_extent;
>>>>>>     	loff_t end;
>>>>>>     	int err;
>>>>>> @@ -3617,6 +3617,8 @@ int f2fs_precache_extents(struct inode *inode)
>>>>>>
>>>>>>     	while (map.m_lblk < end) {
>>>>>>     		map.m_len = end - map.m_lblk;
>>>>>> +		map.m_pblk = 0;
>>>>>> +		map.m_flags = 0;
>>>>>>
>>>>>>     		f2fs_down_write(&fi->i_gc_rwsem[WRITE]);
>>>>>>     		err = f2fs_map_blocks(inode, &map, F2FS_GET_BLOCK_PRECACHE);
>>>>>> -- 
>>>>>> 2.49.0
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>>>     			goto sync_out;
>>>>>>>     		case F2FS_GET_BLOCK_BMAP:
>>>>>>>     			map->m_pblk = 0;
>>>>>
>>>>>
>>>>> We have already reset m_flags (map->m_flags = 0) in f2fs_map_blocks().
>>>>
>>>> Zijie:
>>>>
>>>> Oops, that's right, thanks for correcting me.
>>>>
>>>>>
>>>>> I think that this bug is caused by we missed to reset m_flags when we
>>>>> goto next_dnode in below case：
>>>>>
>>>>> Data layout is something like this:
>>>>> dnode1:                     dnode2:
>>>>> [0]      A                  [0]    NEW_ADDR
>>>>> [1]      A+1                [1]    0x0
>>>>> ...
>>>>> [1016]   A+1016
>>>>> [1017]   B (B!=A+1017)      [1017] 0x0
>>>>>
>>>>> we map the last block(valid blkaddr) in dnode1:
>>>>> map->m_flags |= F2FS_MAP_MAPPED;
>>>>> map->m_pblk = blkaddr(valid blkaddr);
>>>>> map->m_len = 1;
>>>>> then we goto next_dnode, meet the first block in dnode2(hole), goto sync_out:
>>>>> map->m_flags & F2FS_MAP_MAPPED == true, and we make wrong blkaddr/len for extent_info.
>>>>
>>>> So, can you please add above explanation into commit message? that
>>>> should be helpful for understanding the problem more clearly.
>>>>
>>>> Please take a look at this case w/ your patch:
>>>>
>>>> mkfs.f2fs -O extra_attr,compression /dev/vdb -f
>>>> mount /dev/vdb /mnt/f2fs -o mode=lfs
>>>> cd /mnt/f2fs
>>>> f2fs_io write 1 0 1883 rand dsync testfile
>>>> f2fs_io fallocate 0 7712768 4096 testfile
>>>> f2fs_io write 1 1881 1 rand buffered testfile
>>>> xfs_io testfile -c "fsync"
>>>> cd /
>>>> umount /mnt/f2fs
>>>> mount /dev/vdb /mnt/f2fs
>>>> f2fs_io precache_extents /mnt/f2fs/testfile
>>>> umount /mnt/f2fs
>>>>
>>>>            f2fs_io-733     [010] .....    78.134136: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 4, pgofs = 1882, len = 0, blkaddr = 17410, c_len = 0
>>>>
>>>> I suspect we need this?
>>>>
>>>> @@ -1784,7 +1781,8 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>>           }
>>>>
>>>>           if (flag == F2FS_GET_BLOCK_PRECACHE) {
>>>> -               if (map->m_flags & F2FS_MAP_MAPPED) {
>>>> +               if ((map->m_flags & F2FS_MAP_MAPPED) &&
>>>> +                       (map->m_len - ofs)) {
>>>>                           unsigned int ofs = start_pgofs - map->m_lblk;
>>>>
>>>>                           f2fs_update_read_extent_cache_range(&dn,
>>>
>>> Thanks for pointing out this. Let me find a way to cover these cases and do more test.
>>>
>>>> BTW, I find another bug, if one blkaddr is adjcent to previous extent,
>>>> but and it is valid, we need to set m_next_extent to pgofs rather than
>>>> pgofs + 1.
>>>>
>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>> index cbf8841642c7..ac88ed68059c 100644
>>>> --- a/fs/f2fs/data.c
>>>> +++ b/fs/f2fs/data.c
>>>> @@ -1789,8 +1789,11 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>>                                   start_pgofs, map->m_pblk + ofs,
>>>>                                   map->m_len - ofs);
>>>>                   }
>>>> -               if (map->m_next_extent)
>>>> -                       *map->m_next_extent = pgofs + 1;
>>>> +               if (map->m_next_extent) {
>>>> +                       *map->m_next_extent = pgofs;
>>>> +                       if (!__is_valid_data_blkaddr(blkaddr))
>>>> +                               *map->m_next_extent += 1;
>>>> +               }
>>>>           }
>>>>           f2fs_put_dnode(&dn);
>>>
>>> Maybe it can be this?
>>> if (map->m_next_extent)
>>> 	*map->m_next_extent = is_hole ? pgofs + 1 : pgofs;
>>
>> It's better, will update, thank you. :)
>>
>> Thanks,
> 
> Hi Chao,
> I test some cases with this change:
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 7961e0ddf..7093fdc95 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1777,13 +1777,13 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>          if (flag == F2FS_GET_BLOCK_PRECACHE) {
>                  if (map->m_flags & F2FS_MAP_MAPPED) {
>                          unsigned int ofs = start_pgofs - map->m_lblk;
> -
> -                       f2fs_update_read_extent_cache_range(&dn,
> -                               start_pgofs, map->m_pblk + ofs,
> -                               map->m_len - ofs);
> +                       if (map->m_len - ofs > 0)
> +                               f2fs_update_read_extent_cache_range(&dn,
> +                                       start_pgofs, map->m_pblk + ofs,
> +                                       map->m_len - ofs);
>                  }
>                  if (map->m_next_extent)
> -                       *map->m_next_extent = pgofs + 1;
> +                       *map->m_next_extent = is_hole ? pgofs + 1 : pgofs;
>          }
>          f2fs_put_dnode(&dn);
>   unlock_out:
> 
> 
> test cases:
> 
> case1:
> dnode1:                     dnode2:
> [0]      A                  [0]    NEW_ADDR
> [1]      A+1                [1]    0x0
> ...                         ....
> [1016]   A+1016
> [1017]   B (B!=A+1017)      [1017] 0x0
> 
> case2:
> dnode1:                     dnode2:
> [0]      A                  [0]    C (C!=B+1)
> [1]      A+1                [1]    C+1
> ...                         ....
> [1016]   A+1016
> [1017]   B (B!=A+1017)      [1017] 0x0
> 
> case3:
> dnode1:                     dnode2:
> [0]      A                  [0]    C (C!=B+2)
> [1]      A+1                [1]    C+1
> ...                         ....
> [1015]   A+1015
> [1016]   B (B!=A+1016)
> [1017]   B+1                [1017] 0x0
> 
> case4:
> one blkaddr is adjcent to previous extent, and it is valid.
 > > And from the result, it seems this change can cover these
> situations correctly.
> Do we need a patch with this change?

Zijie, thanks for the test.

IMO, we'd better use these changes:

-
-                       f2fs_update_read_extent_cache_range(&dn,
-                               start_pgofs, map->m_pblk + ofs,
-                               map->m_len - ofs);
+                       if (map->m_len - ofs > 0)
+                               f2fs_update_read_extent_cache_range(&dn,
+                                       start_pgofs, map->m_pblk + ofs,
+                                       map->m_len - ofs);

instead of

    		switch (flag) {
    		case F2FS_GET_BLOCK_PRECACHE:
+			if (__is_valid_data_blkaddr(map->m_pblk) &&
+				start_pgofs - map->m_lblk == map->m_len)
+				map->m_flags &= ~F2FS_MAP_MAPPED;

Can you please rebase your patchset on mine and send v2?

https://lore.kernel.org/linux-f2fs-devel/20250912081250.44383-1-chao@kernel.org

BTW, please add fixes line in your patch.

Thanks,



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

Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents

[Chao Yu]

On 9/12/2025 6:06 PM, wangzijie wrote:
>> On 9/12/2025 11:36 AM, wangzijie wrote:
>>>> On 9/11/2025 5:07 PM, wangzijie wrote:
>>>>>> On 9/10/25 21:58, wangzijie wrote:
>>>>>>> When the data layout is like this:
>>>>>>> dnode1:                     dnode2:
>>>>>>> [0]      A                  [0]    NEW_ADDR
>>>>>>> [1]      A+1                [1]    0x0
>>>>>>> ...                         ....
>>>>>>> [1016]   A+1016
>>>>>>> [1017]   B (B!=A+1017)      [1017] 0x0
>>>>>>>
>>>>>>> We can build this kind of layout by following steps(with i_extra_isize:36):
>>>>>>> ./f2fs_io write 1 0 1881 rand dsync testfile
>>>>>>> ./f2fs_io write 1 1881 1 rand buffered testfile
>>>>>>> ./f2fs_io fallocate 0 7708672 4096 testfile
>>>>>>>
>>>>>>> And when we map first data block in dnode2, we will get wrong extent_info data:
>>>>>>> map->m_len = 1
>>>>>>> ofs = start_pgofs - map->m_lblk = 1882 - 1881 = 1
>>>>>>>
>>>>>>> ei.fofs = start_pgofs = 1882
>>>>>>> ei.len = map->m_len - ofs = 1 - 1 = 0
>>>>>>>
>>>>>>> Fix it by skipping updating this kind of extent info.
>>>>>>>
>>>>>>> Signed-off-by: wangzijie <wangzijie1@honor.com>
>>>>>>> ---
>>>>>>>     fs/f2fs/data.c | 3 +++
>>>>>>>     1 file changed, 3 insertions(+)
>>>>>>>
>>>>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>>>>> index 7961e0ddf..b8bb71852 100644
>>>>>>> --- a/fs/f2fs/data.c
>>>>>>> +++ b/fs/f2fs/data.c
>>>>>>> @@ -1649,6 +1649,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>>>>>     
>>>>>>>     		switch (flag) {
>>>>>>>     		case F2FS_GET_BLOCK_PRECACHE:
>>>>>>> +			if (__is_valid_data_blkaddr(map->m_pblk) &&
>>>>>>> +				start_pgofs - map->m_lblk == map->m_len)
>>>>>>> +				map->m_flags &= ~F2FS_MAP_MAPPED;
>>>>>>
>>>>>> It looks we missed to reset value for map variable in f2fs_precache_extents(),
>>>>>> what do you think of this?
>>>>>>
>>>>>> ---
>>>>>>     fs/f2fs/file.c | 4 +++-
>>>>>>     1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>>>>> index 1aae4361d0a8..2b14151d4130 100644
>>>>>> --- a/fs/f2fs/file.c
>>>>>> +++ b/fs/f2fs/file.c
>>>>>> @@ -3599,7 +3599,7 @@ static int f2fs_ioc_io_prio(struct file *filp, unsigned long arg)
>>>>>>     int f2fs_precache_extents(struct inode *inode)
>>>>>>     {
>>>>>>     	struct f2fs_inode_info *fi = F2FS_I(inode);
>>>>>> -	struct f2fs_map_blocks map;
>>>>>> +	struct f2fs_map_blocks map = { 0 };
>>>>>>     	pgoff_t m_next_extent;
>>>>>>     	loff_t end;
>>>>>>     	int err;
>>>>>> @@ -3617,6 +3617,8 @@ int f2fs_precache_extents(struct inode *inode)
>>>>>>
>>>>>>     	while (map.m_lblk < end) {
>>>>>>     		map.m_len = end - map.m_lblk;
>>>>>> +		map.m_pblk = 0;
>>>>>> +		map.m_flags = 0;
>>>>>>
>>>>>>     		f2fs_down_write(&fi->i_gc_rwsem[WRITE]);
>>>>>>     		err = f2fs_map_blocks(inode, &map, F2FS_GET_BLOCK_PRECACHE);
>>>>>> -- 
>>>>>> 2.49.0
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>>>     			goto sync_out;
>>>>>>>     		case F2FS_GET_BLOCK_BMAP:
>>>>>>>     			map->m_pblk = 0;
>>>>>
>>>>>
>>>>> We have already reset m_flags (map->m_flags = 0) in f2fs_map_blocks().
>>>>
>>>> Zijie:
>>>>
>>>> Oops, that's right, thanks for correcting me.
>>>>
>>>>>
>>>>> I think that this bug is caused by we missed to reset m_flags when we
>>>>> goto next_dnode in below case：
>>>>>
>>>>> Data layout is something like this:
>>>>> dnode1:                     dnode2:
>>>>> [0]      A                  [0]    NEW_ADDR
>>>>> [1]      A+1                [1]    0x0
>>>>> ...
>>>>> [1016]   A+1016
>>>>> [1017]   B (B!=A+1017)      [1017] 0x0
>>>>>
>>>>> we map the last block(valid blkaddr) in dnode1:
>>>>> map->m_flags |= F2FS_MAP_MAPPED;
>>>>> map->m_pblk = blkaddr(valid blkaddr);
>>>>> map->m_len = 1;
>>>>> then we goto next_dnode, meet the first block in dnode2(hole), goto sync_out:
>>>>> map->m_flags & F2FS_MAP_MAPPED == true, and we make wrong blkaddr/len for extent_info.
>>>>
>>>> So, can you please add above explanation into commit message? that
>>>> should be helpful for understanding the problem more clearly.
>>>>
>>>> Please take a look at this case w/ your patch:
>>>>
>>>> mkfs.f2fs -O extra_attr,compression /dev/vdb -f
>>>> mount /dev/vdb /mnt/f2fs -o mode=lfs
>>>> cd /mnt/f2fs
>>>> f2fs_io write 1 0 1883 rand dsync testfile
>>>> f2fs_io fallocate 0 7712768 4096 testfile
>>>> f2fs_io write 1 1881 1 rand buffered testfile
>>>> xfs_io testfile -c "fsync"
>>>> cd /
>>>> umount /mnt/f2fs
>>>> mount /dev/vdb /mnt/f2fs
>>>> f2fs_io precache_extents /mnt/f2fs/testfile
>>>> umount /mnt/f2fs
>>>>
>>>>            f2fs_io-733     [010] .....    78.134136: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 4, pgofs = 1882, len = 0, blkaddr = 17410, c_len = 0
>>>>
>>>> I suspect we need this?
>>>>
>>>> @@ -1784,7 +1781,8 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>>           }
>>>>
>>>>           if (flag == F2FS_GET_BLOCK_PRECACHE) {
>>>> -               if (map->m_flags & F2FS_MAP_MAPPED) {
>>>> +               if ((map->m_flags & F2FS_MAP_MAPPED) &&
>>>> +                       (map->m_len - ofs)) {
>>>>                           unsigned int ofs = start_pgofs - map->m_lblk;
>>>>
>>>>                           f2fs_update_read_extent_cache_range(&dn,
>>>
>>> Thanks for pointing out this. Let me find a way to cover these cases and do more test.
>>>
>>>> BTW, I find another bug, if one blkaddr is adjcent to previous extent,
>>>> but and it is valid, we need to set m_next_extent to pgofs rather than
>>>> pgofs + 1.
>>>>
>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>> index cbf8841642c7..ac88ed68059c 100644
>>>> --- a/fs/f2fs/data.c
>>>> +++ b/fs/f2fs/data.c
>>>> @@ -1789,8 +1789,11 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>>                                   start_pgofs, map->m_pblk + ofs,
>>>>                                   map->m_len - ofs);
>>>>                   }
>>>> -               if (map->m_next_extent)
>>>> -                       *map->m_next_extent = pgofs + 1;
>>>> +               if (map->m_next_extent) {
>>>> +                       *map->m_next_extent = pgofs;
>>>> +                       if (!__is_valid_data_blkaddr(blkaddr))
>>>> +                               *map->m_next_extent += 1;
>>>> +               }
>>>>           }
>>>>           f2fs_put_dnode(&dn);
>>>
>>> Maybe it can be this?
>>> if (map->m_next_extent)
>>> 	*map->m_next_extent = is_hole ? pgofs + 1 : pgofs;
>>
>> It's better, will update, thank you. :)
>>
>> Thanks,
> 
> Hi Chao,
> I test some cases with this change:
> 
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 7961e0ddf..7093fdc95 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -1777,13 +1777,13 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>          if (flag == F2FS_GET_BLOCK_PRECACHE) {
>                  if (map->m_flags & F2FS_MAP_MAPPED) {
>                          unsigned int ofs = start_pgofs - map->m_lblk;
> -
> -                       f2fs_update_read_extent_cache_range(&dn,
> -                               start_pgofs, map->m_pblk + ofs,
> -                               map->m_len - ofs);
> +                       if (map->m_len - ofs > 0)
> +                               f2fs_update_read_extent_cache_range(&dn,
> +                                       start_pgofs, map->m_pblk + ofs,
> +                                       map->m_len - ofs);
>                  }
>                  if (map->m_next_extent)
> -                       *map->m_next_extent = pgofs + 1;
> +                       *map->m_next_extent = is_hole ? pgofs + 1 : pgofs;
>          }
>          f2fs_put_dnode(&dn);
>   unlock_out:
> 
> 
> test cases:
> 
> case1:
> dnode1:                     dnode2:
> [0]      A                  [0]    NEW_ADDR
> [1]      A+1                [1]    0x0
> ...                         ....
> [1016]   A+1016
> [1017]   B (B!=A+1017)      [1017] 0x0
> 
> case2:
> dnode1:                     dnode2:
> [0]      A                  [0]    C (C!=B+1)
> [1]      A+1                [1]    C+1
> ...                         ....
> [1016]   A+1016
> [1017]   B (B!=A+1017)      [1017] 0x0
> 
> case3:
> dnode1:                     dnode2:
> [0]      A                  [0]    C (C!=B+2)
> [1]      A+1                [1]    C+1
> ...                         ....
> [1015]   A+1015
> [1016]   B (B!=A+1016)
> [1017]   B+1                [1017] 0x0
> 
> case4:
> one blkaddr is adjcent to previous extent, and it is valid.
 > > And from the result, it seems this change can cover these
> situations correctly.
> Do we need a patch with this change?

Zijie, thanks for the test.

IMO, we'd better use these changes:

-
-                       f2fs_update_read_extent_cache_range(&dn,
-                               start_pgofs, map->m_pblk + ofs,
-                               map->m_len - ofs);
+                       if (map->m_len - ofs > 0)
+                               f2fs_update_read_extent_cache_range(&dn,
+                                       start_pgofs, map->m_pblk + ofs,
+                                       map->m_len - ofs);

instead of

    		switch (flag) {
    		case F2FS_GET_BLOCK_PRECACHE:
+			if (__is_valid_data_blkaddr(map->m_pblk) &&
+				start_pgofs - map->m_lblk == map->m_len)
+				map->m_flags &= ~F2FS_MAP_MAPPED;

Can you please rebase your patchset on mine and send v2?

https://lore.kernel.org/linux-f2fs-devel/20250912081250.44383-1-chao@kernel.org

BTW, please add fixes line in your patch.

Thanks,

Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents

[wangzijie]

>On 9/12/2025 6:06 PM, wangzijie wrote:
>>> On 9/12/2025 11:36 AM, wangzijie wrote:
>>>>> On 9/11/2025 5:07 PM, wangzijie wrote:
>>>>>>> On 9/10/25 21:58, wangzijie wrote:
>>>>>>>> When the data layout is like this:
>>>>>>>> dnode1:                     dnode2:
>>>>>>>> [0]      A                  [0]    NEW_ADDR
>>>>>>>> [1]      A+1                [1]    0x0
>>>>>>>> ...                         ....
>>>>>>>> [1016]   A+1016
>>>>>>>> [1017]   B (B!=A+1017)      [1017] 0x0
>>>>>>>>
>>>>>>>> We can build this kind of layout by following steps(with i_extra_isize:36):
>>>>>>>> ./f2fs_io write 1 0 1881 rand dsync testfile
>>>>>>>> ./f2fs_io write 1 1881 1 rand buffered testfile
>>>>>>>> ./f2fs_io fallocate 0 7708672 4096 testfile
>>>>>>>>
>>>>>>>> And when we map first data block in dnode2, we will get wrong extent_info data:
>>>>>>>> map->m_len = 1
>>>>>>>> ofs = start_pgofs - map->m_lblk = 1882 - 1881 = 1
>>>>>>>>
>>>>>>>> ei.fofs = start_pgofs = 1882
>>>>>>>> ei.len = map->m_len - ofs = 1 - 1 = 0
>>>>>>>>
>>>>>>>> Fix it by skipping updating this kind of extent info.
>>>>>>>>
>>>>>>>> Signed-off-by: wangzijie <wangzijie1@honor.com>
>>>>>>>> ---
>>>>>>>>     fs/f2fs/data.c | 3 +++
>>>>>>>>     1 file changed, 3 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>>>>>> index 7961e0ddf..b8bb71852 100644
>>>>>>>> --- a/fs/f2fs/data.c
>>>>>>>> +++ b/fs/f2fs/data.c
>>>>>>>> @@ -1649,6 +1649,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>>>>>>     
>>>>>>>>     		switch (flag) {
>>>>>>>>     		case F2FS_GET_BLOCK_PRECACHE:
>>>>>>>> +			if (__is_valid_data_blkaddr(map->m_pblk) &&
>>>>>>>> +				start_pgofs - map->m_lblk == map->m_len)
>>>>>>>> +				map->m_flags &= ~F2FS_MAP_MAPPED;
>>>>>>>
>>>>>>> It looks we missed to reset value for map variable in f2fs_precache_extents(),
>>>>>>> what do you think of this?
>>>>>>>
>>>>>>> ---
>>>>>>>     fs/f2fs/file.c | 4 +++-
>>>>>>>     1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>>>>>> index 1aae4361d0a8..2b14151d4130 100644
>>>>>>> --- a/fs/f2fs/file.c
>>>>>>> +++ b/fs/f2fs/file.c
>>>>>>> @@ -3599,7 +3599,7 @@ static int f2fs_ioc_io_prio(struct file *filp, unsigned long arg)
>>>>>>>     int f2fs_precache_extents(struct inode *inode)
>>>>>>>     {
>>>>>>>     	struct f2fs_inode_info *fi = F2FS_I(inode);
>>>>>>> -	struct f2fs_map_blocks map;
>>>>>>> +	struct f2fs_map_blocks map = { 0 };
>>>>>>>     	pgoff_t m_next_extent;
>>>>>>>     	loff_t end;
>>>>>>>     	int err;
>>>>>>> @@ -3617,6 +3617,8 @@ int f2fs_precache_extents(struct inode *inode)
>>>>>>>
>>>>>>>     	while (map.m_lblk < end) {
>>>>>>>     		map.m_len = end - map.m_lblk;
>>>>>>> +		map.m_pblk = 0;
>>>>>>> +		map.m_flags = 0;
>>>>>>>
>>>>>>>     		f2fs_down_write(&fi->i_gc_rwsem[WRITE]);
>>>>>>>     		err = f2fs_map_blocks(inode, &map, F2FS_GET_BLOCK_PRECACHE);
>>>>>>> -- 
>>>>>>> 2.49.0
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>>>     			goto sync_out;
>>>>>>>>     		case F2FS_GET_BLOCK_BMAP:
>>>>>>>>     			map->m_pblk = 0;
>>>>>>
>>>>>>
>>>>>> We have already reset m_flags (map->m_flags = 0) in f2fs_map_blocks().
>>>>>
>>>>> Zijie:
>>>>>
>>>>> Oops, that's right, thanks for correcting me.
>>>>>
>>>>>>
>>>>>> I think that this bug is caused by we missed to reset m_flags when we
>>>>>> goto next_dnode in below case：
>>>>>>
>>>>>> Data layout is something like this:
>>>>>> dnode1:                     dnode2:
>>>>>> [0]      A                  [0]    NEW_ADDR
>>>>>> [1]      A+1                [1]    0x0
>>>>>> ...
>>>>>> [1016]   A+1016
>>>>>> [1017]   B (B!=A+1017)      [1017] 0x0
>>>>>>
>>>>>> we map the last block(valid blkaddr) in dnode1:
>>>>>> map->m_flags |= F2FS_MAP_MAPPED;
>>>>>> map->m_pblk = blkaddr(valid blkaddr);
>>>>>> map->m_len = 1;
>>>>>> then we goto next_dnode, meet the first block in dnode2(hole), goto sync_out:
>>>>>> map->m_flags & F2FS_MAP_MAPPED == true, and we make wrong blkaddr/len for extent_info.
>>>>>
>>>>> So, can you please add above explanation into commit message? that
>>>>> should be helpful for understanding the problem more clearly.
>>>>>
>>>>> Please take a look at this case w/ your patch:
>>>>>
>>>>> mkfs.f2fs -O extra_attr,compression /dev/vdb -f
>>>>> mount /dev/vdb /mnt/f2fs -o mode=lfs
>>>>> cd /mnt/f2fs
>>>>> f2fs_io write 1 0 1883 rand dsync testfile
>>>>> f2fs_io fallocate 0 7712768 4096 testfile
>>>>> f2fs_io write 1 1881 1 rand buffered testfile
>>>>> xfs_io testfile -c "fsync"
>>>>> cd /
>>>>> umount /mnt/f2fs
>>>>> mount /dev/vdb /mnt/f2fs
>>>>> f2fs_io precache_extents /mnt/f2fs/testfile
>>>>> umount /mnt/f2fs
>>>>>
>>>>>            f2fs_io-733     [010] .....    78.134136: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 4, pgofs = 1882, len = 0, blkaddr = 17410, c_len = 0
>>>>>
>>>>> I suspect we need this?
>>>>>
>>>>> @@ -1784,7 +1781,8 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>>>           }
>>>>>
>>>>>           if (flag == F2FS_GET_BLOCK_PRECACHE) {
>>>>> -               if (map->m_flags & F2FS_MAP_MAPPED) {
>>>>> +               if ((map->m_flags & F2FS_MAP_MAPPED) &&
>>>>> +                       (map->m_len - ofs)) {
>>>>>                           unsigned int ofs = start_pgofs - map->m_lblk;
>>>>>
>>>>>                           f2fs_update_read_extent_cache_range(&dn,
>>>>
>>>> Thanks for pointing out this. Let me find a way to cover these cases and do more test.
>>>>
>>>>> BTW, I find another bug, if one blkaddr is adjcent to previous extent,
>>>>> but and it is valid, we need to set m_next_extent to pgofs rather than
>>>>> pgofs + 1.
>>>>>
>>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>>> index cbf8841642c7..ac88ed68059c 100644
>>>>> --- a/fs/f2fs/data.c
>>>>> +++ b/fs/f2fs/data.c
>>>>> @@ -1789,8 +1789,11 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>>>                                   start_pgofs, map->m_pblk + ofs,
>>>>>                                   map->m_len - ofs);
>>>>>                   }
>>>>> -               if (map->m_next_extent)
>>>>> -                       *map->m_next_extent = pgofs + 1;
>>>>> +               if (map->m_next_extent) {
>>>>> +                       *map->m_next_extent = pgofs;
>>>>> +                       if (!__is_valid_data_blkaddr(blkaddr))
>>>>> +                               *map->m_next_extent += 1;
>>>>> +               }
>>>>>           }
>>>>>           f2fs_put_dnode(&dn);
>>>>
>>>> Maybe it can be this?
>>>> if (map->m_next_extent)
>>>> 	*map->m_next_extent = is_hole ? pgofs + 1 : pgofs;
>>>
>>> It's better, will update, thank you. :)
>>>
>>> Thanks,
>> 
>> Hi Chao,
>> I test some cases with this change:
>> 
>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>> index 7961e0ddf..7093fdc95 100644
>> --- a/fs/f2fs/data.c
>> +++ b/fs/f2fs/data.c
>> @@ -1777,13 +1777,13 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>          if (flag == F2FS_GET_BLOCK_PRECACHE) {
>>                  if (map->m_flags & F2FS_MAP_MAPPED) {
>>                          unsigned int ofs = start_pgofs - map->m_lblk;
>> -
>> -                       f2fs_update_read_extent_cache_range(&dn,
>> -                               start_pgofs, map->m_pblk + ofs,
>> -                               map->m_len - ofs);
>> +                       if (map->m_len - ofs > 0)
>> +                               f2fs_update_read_extent_cache_range(&dn,
>> +                                       start_pgofs, map->m_pblk + ofs,
>> +                                       map->m_len - ofs);
>>                  }
>>                  if (map->m_next_extent)
>> -                       *map->m_next_extent = pgofs + 1;
>> +                       *map->m_next_extent = is_hole ? pgofs + 1 : pgofs;
>>          }
>>          f2fs_put_dnode(&dn);
>>   unlock_out:
>> 
>> 
>> test cases:
>> 
>> case1:
>> dnode1:                     dnode2:
>> [0]      A                  [0]    NEW_ADDR
>> [1]      A+1                [1]    0x0
>> ...                         ....
>> [1016]   A+1016
>> [1017]   B (B!=A+1017)      [1017] 0x0
>> 
>> case2:
>> dnode1:                     dnode2:
>> [0]      A                  [0]    C (C!=B+1)
>> [1]      A+1                [1]    C+1
>> ...                         ....
>> [1016]   A+1016
>> [1017]   B (B!=A+1017)      [1017] 0x0
>> 
>> case3:
>> dnode1:                     dnode2:
>> [0]      A                  [0]    C (C!=B+2)
>> [1]      A+1                [1]    C+1
>> ...                         ....
>> [1015]   A+1015
>> [1016]   B (B!=A+1016)
>> [1017]   B+1                [1017] 0x0
>> 
>> case4:
>> one blkaddr is adjcent to previous extent, and it is valid.
> > > And from the result, it seems this change can cover these
>> situations correctly.
>> Do we need a patch with this change?
>
>Zijie, thanks for the test.
>
>IMO, we'd better use these changes:
>
>-
>-                       f2fs_update_read_extent_cache_range(&dn,
>-                               start_pgofs, map->m_pblk + ofs,
>-                               map->m_len - ofs);
>+                       if (map->m_len - ofs > 0)
>+                               f2fs_update_read_extent_cache_range(&dn,
>+                                       start_pgofs, map->m_pblk + ofs,
>+                                       map->m_len - ofs);
>
>instead of
>
>    		switch (flag) {
>    		case F2FS_GET_BLOCK_PRECACHE:
>+			if (__is_valid_data_blkaddr(map->m_pblk) &&
>+				start_pgofs - map->m_lblk == map->m_len)
>+				map->m_flags &= ~F2FS_MAP_MAPPED;
>
>Can you please rebase your patchset on mine and send v2?
>
>https://lore.kernel.org/linux-f2fs-devel/20250912081250.44383-1-chao@kernel.org
>
>BTW, please add fixes line in your patch.
>
>Thanks,

OK, I will correct this part and follow your suggestion. Thank you.


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

Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents

[wangzijie]

>On 9/12/2025 6:06 PM, wangzijie wrote:
>>> On 9/12/2025 11:36 AM, wangzijie wrote:
>>>>> On 9/11/2025 5:07 PM, wangzijie wrote:
>>>>>>> On 9/10/25 21:58, wangzijie wrote:
>>>>>>>> When the data layout is like this:
>>>>>>>> dnode1:                     dnode2:
>>>>>>>> [0]      A                  [0]    NEW_ADDR
>>>>>>>> [1]      A+1                [1]    0x0
>>>>>>>> ...                         ....
>>>>>>>> [1016]   A+1016
>>>>>>>> [1017]   B (B!=A+1017)      [1017] 0x0
>>>>>>>>
>>>>>>>> We can build this kind of layout by following steps(with i_extra_isize:36):
>>>>>>>> ./f2fs_io write 1 0 1881 rand dsync testfile
>>>>>>>> ./f2fs_io write 1 1881 1 rand buffered testfile
>>>>>>>> ./f2fs_io fallocate 0 7708672 4096 testfile
>>>>>>>>
>>>>>>>> And when we map first data block in dnode2, we will get wrong extent_info data:
>>>>>>>> map->m_len = 1
>>>>>>>> ofs = start_pgofs - map->m_lblk = 1882 - 1881 = 1
>>>>>>>>
>>>>>>>> ei.fofs = start_pgofs = 1882
>>>>>>>> ei.len = map->m_len - ofs = 1 - 1 = 0
>>>>>>>>
>>>>>>>> Fix it by skipping updating this kind of extent info.
>>>>>>>>
>>>>>>>> Signed-off-by: wangzijie <wangzijie1@honor.com>
>>>>>>>> ---
>>>>>>>>     fs/f2fs/data.c | 3 +++
>>>>>>>>     1 file changed, 3 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>>>>>> index 7961e0ddf..b8bb71852 100644
>>>>>>>> --- a/fs/f2fs/data.c
>>>>>>>> +++ b/fs/f2fs/data.c
>>>>>>>> @@ -1649,6 +1649,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>>>>>>     
>>>>>>>>     		switch (flag) {
>>>>>>>>     		case F2FS_GET_BLOCK_PRECACHE:
>>>>>>>> +			if (__is_valid_data_blkaddr(map->m_pblk) &&
>>>>>>>> +				start_pgofs - map->m_lblk == map->m_len)
>>>>>>>> +				map->m_flags &= ~F2FS_MAP_MAPPED;
>>>>>>>
>>>>>>> It looks we missed to reset value for map variable in f2fs_precache_extents(),
>>>>>>> what do you think of this?
>>>>>>>
>>>>>>> ---
>>>>>>>     fs/f2fs/file.c | 4 +++-
>>>>>>>     1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>>>>>> index 1aae4361d0a8..2b14151d4130 100644
>>>>>>> --- a/fs/f2fs/file.c
>>>>>>> +++ b/fs/f2fs/file.c
>>>>>>> @@ -3599,7 +3599,7 @@ static int f2fs_ioc_io_prio(struct file *filp, unsigned long arg)
>>>>>>>     int f2fs_precache_extents(struct inode *inode)
>>>>>>>     {
>>>>>>>     	struct f2fs_inode_info *fi = F2FS_I(inode);
>>>>>>> -	struct f2fs_map_blocks map;
>>>>>>> +	struct f2fs_map_blocks map = { 0 };
>>>>>>>     	pgoff_t m_next_extent;
>>>>>>>     	loff_t end;
>>>>>>>     	int err;
>>>>>>> @@ -3617,6 +3617,8 @@ int f2fs_precache_extents(struct inode *inode)
>>>>>>>
>>>>>>>     	while (map.m_lblk < end) {
>>>>>>>     		map.m_len = end - map.m_lblk;
>>>>>>> +		map.m_pblk = 0;
>>>>>>> +		map.m_flags = 0;
>>>>>>>
>>>>>>>     		f2fs_down_write(&fi->i_gc_rwsem[WRITE]);
>>>>>>>     		err = f2fs_map_blocks(inode, &map, F2FS_GET_BLOCK_PRECACHE);
>>>>>>> -- 
>>>>>>> 2.49.0
>>>>>>>
>>>>>>> Thanks,
>>>>>>>
>>>>>>>>     			goto sync_out;
>>>>>>>>     		case F2FS_GET_BLOCK_BMAP:
>>>>>>>>     			map->m_pblk = 0;
>>>>>>
>>>>>>
>>>>>> We have already reset m_flags (map->m_flags = 0) in f2fs_map_blocks().
>>>>>
>>>>> Zijie:
>>>>>
>>>>> Oops, that's right, thanks for correcting me.
>>>>>
>>>>>>
>>>>>> I think that this bug is caused by we missed to reset m_flags when we
>>>>>> goto next_dnode in below case：
>>>>>>
>>>>>> Data layout is something like this:
>>>>>> dnode1:                     dnode2:
>>>>>> [0]      A                  [0]    NEW_ADDR
>>>>>> [1]      A+1                [1]    0x0
>>>>>> ...
>>>>>> [1016]   A+1016
>>>>>> [1017]   B (B!=A+1017)      [1017] 0x0
>>>>>>
>>>>>> we map the last block(valid blkaddr) in dnode1:
>>>>>> map->m_flags |= F2FS_MAP_MAPPED;
>>>>>> map->m_pblk = blkaddr(valid blkaddr);
>>>>>> map->m_len = 1;
>>>>>> then we goto next_dnode, meet the first block in dnode2(hole), goto sync_out:
>>>>>> map->m_flags & F2FS_MAP_MAPPED == true, and we make wrong blkaddr/len for extent_info.
>>>>>
>>>>> So, can you please add above explanation into commit message? that
>>>>> should be helpful for understanding the problem more clearly.
>>>>>
>>>>> Please take a look at this case w/ your patch:
>>>>>
>>>>> mkfs.f2fs -O extra_attr,compression /dev/vdb -f
>>>>> mount /dev/vdb /mnt/f2fs -o mode=lfs
>>>>> cd /mnt/f2fs
>>>>> f2fs_io write 1 0 1883 rand dsync testfile
>>>>> f2fs_io fallocate 0 7712768 4096 testfile
>>>>> f2fs_io write 1 1881 1 rand buffered testfile
>>>>> xfs_io testfile -c "fsync"
>>>>> cd /
>>>>> umount /mnt/f2fs
>>>>> mount /dev/vdb /mnt/f2fs
>>>>> f2fs_io precache_extents /mnt/f2fs/testfile
>>>>> umount /mnt/f2fs
>>>>>
>>>>>            f2fs_io-733     [010] .....    78.134136: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 4, pgofs = 1882, len = 0, blkaddr = 17410, c_len = 0
>>>>>
>>>>> I suspect we need this?
>>>>>
>>>>> @@ -1784,7 +1781,8 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>>>           }
>>>>>
>>>>>           if (flag == F2FS_GET_BLOCK_PRECACHE) {
>>>>> -               if (map->m_flags & F2FS_MAP_MAPPED) {
>>>>> +               if ((map->m_flags & F2FS_MAP_MAPPED) &&
>>>>> +                       (map->m_len - ofs)) {
>>>>>                           unsigned int ofs = start_pgofs - map->m_lblk;
>>>>>
>>>>>                           f2fs_update_read_extent_cache_range(&dn,
>>>>
>>>> Thanks for pointing out this. Let me find a way to cover these cases and do more test.
>>>>
>>>>> BTW, I find another bug, if one blkaddr is adjcent to previous extent,
>>>>> but and it is valid, we need to set m_next_extent to pgofs rather than
>>>>> pgofs + 1.
>>>>>
>>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>>> index cbf8841642c7..ac88ed68059c 100644
>>>>> --- a/fs/f2fs/data.c
>>>>> +++ b/fs/f2fs/data.c
>>>>> @@ -1789,8 +1789,11 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>>>                                   start_pgofs, map->m_pblk + ofs,
>>>>>                                   map->m_len - ofs);
>>>>>                   }
>>>>> -               if (map->m_next_extent)
>>>>> -                       *map->m_next_extent = pgofs + 1;
>>>>> +               if (map->m_next_extent) {
>>>>> +                       *map->m_next_extent = pgofs;
>>>>> +                       if (!__is_valid_data_blkaddr(blkaddr))
>>>>> +                               *map->m_next_extent += 1;
>>>>> +               }
>>>>>           }
>>>>>           f2fs_put_dnode(&dn);
>>>>
>>>> Maybe it can be this?
>>>> if (map->m_next_extent)
>>>> 	*map->m_next_extent = is_hole ? pgofs + 1 : pgofs;
>>>
>>> It's better, will update, thank you. :)
>>>
>>> Thanks,
>> 
>> Hi Chao,
>> I test some cases with this change:
>> 
>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>> index 7961e0ddf..7093fdc95 100644
>> --- a/fs/f2fs/data.c
>> +++ b/fs/f2fs/data.c
>> @@ -1777,13 +1777,13 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>          if (flag == F2FS_GET_BLOCK_PRECACHE) {
>>                  if (map->m_flags & F2FS_MAP_MAPPED) {
>>                          unsigned int ofs = start_pgofs - map->m_lblk;
>> -
>> -                       f2fs_update_read_extent_cache_range(&dn,
>> -                               start_pgofs, map->m_pblk + ofs,
>> -                               map->m_len - ofs);
>> +                       if (map->m_len - ofs > 0)
>> +                               f2fs_update_read_extent_cache_range(&dn,
>> +                                       start_pgofs, map->m_pblk + ofs,
>> +                                       map->m_len - ofs);
>>                  }
>>                  if (map->m_next_extent)
>> -                       *map->m_next_extent = pgofs + 1;
>> +                       *map->m_next_extent = is_hole ? pgofs + 1 : pgofs;
>>          }
>>          f2fs_put_dnode(&dn);
>>   unlock_out:
>> 
>> 
>> test cases:
>> 
>> case1:
>> dnode1:                     dnode2:
>> [0]      A                  [0]    NEW_ADDR
>> [1]      A+1                [1]    0x0
>> ...                         ....
>> [1016]   A+1016
>> [1017]   B (B!=A+1017)      [1017] 0x0
>> 
>> case2:
>> dnode1:                     dnode2:
>> [0]      A                  [0]    C (C!=B+1)
>> [1]      A+1                [1]    C+1
>> ...                         ....
>> [1016]   A+1016
>> [1017]   B (B!=A+1017)      [1017] 0x0
>> 
>> case3:
>> dnode1:                     dnode2:
>> [0]      A                  [0]    C (C!=B+2)
>> [1]      A+1                [1]    C+1
>> ...                         ....
>> [1015]   A+1015
>> [1016]   B (B!=A+1016)
>> [1017]   B+1                [1017] 0x0
>> 
>> case4:
>> one blkaddr is adjcent to previous extent, and it is valid.
> > > And from the result, it seems this change can cover these
>> situations correctly.
>> Do we need a patch with this change?
>
>Zijie, thanks for the test.
>
>IMO, we'd better use these changes:
>
>-
>-                       f2fs_update_read_extent_cache_range(&dn,
>-                               start_pgofs, map->m_pblk + ofs,
>-                               map->m_len - ofs);
>+                       if (map->m_len - ofs > 0)
>+                               f2fs_update_read_extent_cache_range(&dn,
>+                                       start_pgofs, map->m_pblk + ofs,
>+                                       map->m_len - ofs);
>
>instead of
>
>    		switch (flag) {
>    		case F2FS_GET_BLOCK_PRECACHE:
>+			if (__is_valid_data_blkaddr(map->m_pblk) &&
>+				start_pgofs - map->m_lblk == map->m_len)
>+				map->m_flags &= ~F2FS_MAP_MAPPED;
>
>Can you please rebase your patchset on mine and send v2?
>
>https://lore.kernel.org/linux-f2fs-devel/20250912081250.44383-1-chao@kernel.org
>
>BTW, please add fixes line in your patch.
>
>Thanks,

OK, I will correct this part and follow your suggestion. Thank you.

Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents

[wangzijie]

>>On 9/12/2025 11:36 AM, wangzijie wrote:
>>>> On 9/11/2025 5:07 PM, wangzijie wrote:
>>>>>> On 9/10/25 21:58, wangzijie wrote:
>>>>>>> When the data layout is like this:
>>>>>>> dnode1:                     dnode2:
>>>>>>> [0]      A                  [0]    NEW_ADDR
>>>>>>> [1]      A+1                [1]    0x0
>>>>>>> ...                         ....
>>>>>>> [1016]   A+1016
>>>>>>> [1017]   B (B!=A+1017)      [1017] 0x0
>>>>>>>
>>>>>>> We can build this kind of layout by following steps(with i_extra_isize:36):
>>>>>>> ./f2fs_io write 1 0 1881 rand dsync testfile
>>>>>>> ./f2fs_io write 1 1881 1 rand buffered testfile
>>>>>>> ./f2fs_io fallocate 0 7708672 4096 testfile
>>>>>>>
>>>>>>> And when we map first data block in dnode2, we will get wrong extent_info data:
>>>>>>> map->m_len = 1
>>>>>>> ofs = start_pgofs - map->m_lblk = 1882 - 1881 = 1
>>>>>>>
>>>>>>> ei.fofs = start_pgofs = 1882
>>>>>>> ei.len = map->m_len - ofs = 1 - 1 = 0
>>>>>>>
>>>>>>> Fix it by skipping updating this kind of extent info.
>>>>>>>
>>>>>>> Signed-off-by: wangzijie <wangzijie1@honor.com>
>>>>>>> ---
>>>>>>>    fs/f2fs/data.c | 3 +++
>>>>>>>    1 file changed, 3 insertions(+)
>>>>>>>
>>>>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>>>>> index 7961e0ddf..b8bb71852 100644
>>>>>>> --- a/fs/f2fs/data.c
>>>>>>> +++ b/fs/f2fs/data.c
>>>>>>> @@ -1649,6 +1649,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>>>>>    
>>>>>>>    		switch (flag) {
>>>>>>>    		case F2FS_GET_BLOCK_PRECACHE:
>>>>>>> +			if (__is_valid_data_blkaddr(map->m_pblk) &&
>>>>>>> +				start_pgofs - map->m_lblk == map->m_len)
>>>>>>> +				map->m_flags &= ~F2FS_MAP_MAPPED;
>>>>>>
>>>>>> It looks we missed to reset value for map variable in f2fs_precache_extents(),
>>>>>> what do you think of this?
>>>>>>
>>>>>> ---
>>>>>>    fs/f2fs/file.c | 4 +++-
>>>>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>>>>> index 1aae4361d0a8..2b14151d4130 100644
>>>>>> --- a/fs/f2fs/file.c
>>>>>> +++ b/fs/f2fs/file.c
>>>>>> @@ -3599,7 +3599,7 @@ static int f2fs_ioc_io_prio(struct file *filp, unsigned long arg)
>>>>>>    int f2fs_precache_extents(struct inode *inode)
>>>>>>    {
>>>>>>    	struct f2fs_inode_info *fi = F2FS_I(inode);
>>>>>> -	struct f2fs_map_blocks map;
>>>>>> +	struct f2fs_map_blocks map = { 0 };
>>>>>>    	pgoff_t m_next_extent;
>>>>>>    	loff_t end;
>>>>>>    	int err;
>>>>>> @@ -3617,6 +3617,8 @@ int f2fs_precache_extents(struct inode *inode)
>>>>>>
>>>>>>    	while (map.m_lblk < end) {
>>>>>>    		map.m_len = end - map.m_lblk;
>>>>>> +		map.m_pblk = 0;
>>>>>> +		map.m_flags = 0;
>>>>>>
>>>>>>    		f2fs_down_write(&fi->i_gc_rwsem[WRITE]);
>>>>>>    		err = f2fs_map_blocks(inode, &map, F2FS_GET_BLOCK_PRECACHE);
>>>>>> -- 
>>>>>> 2.49.0
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>>>    			goto sync_out;
>>>>>>>    		case F2FS_GET_BLOCK_BMAP:
>>>>>>>    			map->m_pblk = 0;
>>>>>
>>>>>
>>>>> We have already reset m_flags (map->m_flags = 0) in f2fs_map_blocks().
>>>>
>>>> Zijie:
>>>>
>>>> Oops, that's right, thanks for correcting me.
>>>>
>>>>>
>>>>> I think that this bug is caused by we missed to reset m_flags when we
>>>>> goto next_dnode in below case：
>>>>>
>>>>> Data layout is something like this:
>>>>> dnode1:                     dnode2:
>>>>> [0]      A                  [0]    NEW_ADDR
>>>>> [1]      A+1                [1]    0x0
>>>>> ...
>>>>> [1016]   A+1016
>>>>> [1017]   B (B!=A+1017)      [1017] 0x0
>>>>>
>>>>> we map the last block(valid blkaddr) in dnode1:
>>>>> map->m_flags |= F2FS_MAP_MAPPED;
>>>>> map->m_pblk = blkaddr(valid blkaddr);
>>>>> map->m_len = 1;
>>>>> then we goto next_dnode, meet the first block in dnode2(hole), goto sync_out:
>>>>> map->m_flags & F2FS_MAP_MAPPED == true, and we make wrong blkaddr/len for extent_info.
>>>>
>>>> So, can you please add above explanation into commit message? that
>>>> should be helpful for understanding the problem more clearly.
>>>>
>>>> Please take a look at this case w/ your patch:
>>>>
>>>> mkfs.f2fs -O extra_attr,compression /dev/vdb -f
>>>> mount /dev/vdb /mnt/f2fs -o mode=lfs
>>>> cd /mnt/f2fs
>>>> f2fs_io write 1 0 1883 rand dsync testfile
>>>> f2fs_io fallocate 0 7712768 4096 testfile
>>>> f2fs_io write 1 1881 1 rand buffered testfile
>>>> xfs_io testfile -c "fsync"
>>>> cd /
>>>> umount /mnt/f2fs
>>>> mount /dev/vdb /mnt/f2fs
>>>> f2fs_io precache_extents /mnt/f2fs/testfile
>>>> umount /mnt/f2fs
>>>>
>>>>           f2fs_io-733     [010] .....    78.134136: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 4, pgofs = 1882, len = 0, blkaddr = 17410, c_len = 0
>>>>
>>>> I suspect we need this?
>>>>
>>>> @@ -1784,7 +1781,8 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>>          }
>>>>
>>>>          if (flag == F2FS_GET_BLOCK_PRECACHE) {
>>>> -               if (map->m_flags & F2FS_MAP_MAPPED) {
>>>> +               if ((map->m_flags & F2FS_MAP_MAPPED) &&
>>>> +                       (map->m_len - ofs)) {
>>>>                          unsigned int ofs = start_pgofs - map->m_lblk;
>>>>
>>>>                          f2fs_update_read_extent_cache_range(&dn,
>>> 
>>> Thanks for pointing out this. Let me find a way to cover these cases and do more test.
>>> 
>>>> BTW, I find another bug, if one blkaddr is adjcent to previous extent,
>>>> but and it is valid, we need to set m_next_extent to pgofs rather than
>>>> pgofs + 1.
>>>>
>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>> index cbf8841642c7..ac88ed68059c 100644
>>>> --- a/fs/f2fs/data.c
>>>> +++ b/fs/f2fs/data.c
>>>> @@ -1789,8 +1789,11 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>>                                  start_pgofs, map->m_pblk + ofs,
>>>>                                  map->m_len - ofs);
>>>>                  }
>>>> -               if (map->m_next_extent)
>>>> -                       *map->m_next_extent = pgofs + 1;
>>>> +               if (map->m_next_extent) {
>>>> +                       *map->m_next_extent = pgofs;
>>>> +                       if (!__is_valid_data_blkaddr(blkaddr))
>>>> +                               *map->m_next_extent += 1;
>>>> +               }
>>>>          }
>>>>          f2fs_put_dnode(&dn);
>>> 
>>> Maybe it can be this?
>>> if (map->m_next_extent)
>>> 	*map->m_next_extent = is_hole ? pgofs + 1 : pgofs;
>>
>>It's better, will update, thank you. :)
>>
>>Thanks,
>
>Hi Chao,
>I test some cases with this change:
>
>diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>index 7961e0ddf..7093fdc95 100644
>--- a/fs/f2fs/data.c
>+++ b/fs/f2fs/data.c
>@@ -1777,13 +1777,13 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>        if (flag == F2FS_GET_BLOCK_PRECACHE) {
>                if (map->m_flags & F2FS_MAP_MAPPED) {
>                        unsigned int ofs = start_pgofs - map->m_lblk;
>-
>-                       f2fs_update_read_extent_cache_range(&dn,
>-                               start_pgofs, map->m_pblk + ofs,
>-                               map->m_len - ofs);
>+                       if (map->m_len - ofs > 0)
>+                               f2fs_update_read_extent_cache_range(&dn,
>+                                       start_pgofs, map->m_pblk + ofs,
>+                                       map->m_len - ofs);
>                }
>                if (map->m_next_extent)
>-                       *map->m_next_extent = pgofs + 1;
>+                       *map->m_next_extent = is_hole ? pgofs + 1 : pgofs;
>        }
>        f2fs_put_dnode(&dn);
> unlock_out:
>
>
>test cases:
>
>case1:
>dnode1:                     dnode2:
>[0]      A                  [0]    NEW_ADDR
>[1]      A+1                [1]    0x0
>...                         ....
>[1016]   A+1016
>[1017]   B (B!=A+1017)      [1017] 0x0
>
>case2:
>dnode1:                     dnode2:
>[0]      A                  [0]    C (C!=B+1)
>[1]      A+1                [1]    C+1
>...                         ....
>[1016]   A+1016
>[1017]   B (B!=A+1017)      [1017] 0x0
>
>case3:
>dnode1:                     dnode2:
>[0]      A                  [0]    C (C!=B+2)
>[1]      A+1                [1]    C+1
>...                         ....
>[1015]   A+1015
>[1016]   B (B!=A+1016)
>[1017]   B+1                [1017] 0x0
>
>case4:
>one blkaddr is adjcent to previous extent, and it is valid.
>
>And from the result, it seems this change can cover these
>situations correctly.
>Do we need a patch with this change?

Sorry, with this change, for case1:
The first block of dnode2 ([0]:NEW_ADDR) will be skipped.
Let me find a better way....


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

Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents

[wangzijie]

>>On 9/12/2025 11:36 AM, wangzijie wrote:
>>>> On 9/11/2025 5:07 PM, wangzijie wrote:
>>>>>> On 9/10/25 21:58, wangzijie wrote:
>>>>>>> When the data layout is like this:
>>>>>>> dnode1:                     dnode2:
>>>>>>> [0]      A                  [0]    NEW_ADDR
>>>>>>> [1]      A+1                [1]    0x0
>>>>>>> ...                         ....
>>>>>>> [1016]   A+1016
>>>>>>> [1017]   B (B!=A+1017)      [1017] 0x0
>>>>>>>
>>>>>>> We can build this kind of layout by following steps(with i_extra_isize:36):
>>>>>>> ./f2fs_io write 1 0 1881 rand dsync testfile
>>>>>>> ./f2fs_io write 1 1881 1 rand buffered testfile
>>>>>>> ./f2fs_io fallocate 0 7708672 4096 testfile
>>>>>>>
>>>>>>> And when we map first data block in dnode2, we will get wrong extent_info data:
>>>>>>> map->m_len = 1
>>>>>>> ofs = start_pgofs - map->m_lblk = 1882 - 1881 = 1
>>>>>>>
>>>>>>> ei.fofs = start_pgofs = 1882
>>>>>>> ei.len = map->m_len - ofs = 1 - 1 = 0
>>>>>>>
>>>>>>> Fix it by skipping updating this kind of extent info.
>>>>>>>
>>>>>>> Signed-off-by: wangzijie <wangzijie1@honor.com>
>>>>>>> ---
>>>>>>>    fs/f2fs/data.c | 3 +++
>>>>>>>    1 file changed, 3 insertions(+)
>>>>>>>
>>>>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>>>>> index 7961e0ddf..b8bb71852 100644
>>>>>>> --- a/fs/f2fs/data.c
>>>>>>> +++ b/fs/f2fs/data.c
>>>>>>> @@ -1649,6 +1649,9 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>>>>>    
>>>>>>>    		switch (flag) {
>>>>>>>    		case F2FS_GET_BLOCK_PRECACHE:
>>>>>>> +			if (__is_valid_data_blkaddr(map->m_pblk) &&
>>>>>>> +				start_pgofs - map->m_lblk == map->m_len)
>>>>>>> +				map->m_flags &= ~F2FS_MAP_MAPPED;
>>>>>>
>>>>>> It looks we missed to reset value for map variable in f2fs_precache_extents(),
>>>>>> what do you think of this?
>>>>>>
>>>>>> ---
>>>>>>    fs/f2fs/file.c | 4 +++-
>>>>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
>>>>>> index 1aae4361d0a8..2b14151d4130 100644
>>>>>> --- a/fs/f2fs/file.c
>>>>>> +++ b/fs/f2fs/file.c
>>>>>> @@ -3599,7 +3599,7 @@ static int f2fs_ioc_io_prio(struct file *filp, unsigned long arg)
>>>>>>    int f2fs_precache_extents(struct inode *inode)
>>>>>>    {
>>>>>>    	struct f2fs_inode_info *fi = F2FS_I(inode);
>>>>>> -	struct f2fs_map_blocks map;
>>>>>> +	struct f2fs_map_blocks map = { 0 };
>>>>>>    	pgoff_t m_next_extent;
>>>>>>    	loff_t end;
>>>>>>    	int err;
>>>>>> @@ -3617,6 +3617,8 @@ int f2fs_precache_extents(struct inode *inode)
>>>>>>
>>>>>>    	while (map.m_lblk < end) {
>>>>>>    		map.m_len = end - map.m_lblk;
>>>>>> +		map.m_pblk = 0;
>>>>>> +		map.m_flags = 0;
>>>>>>
>>>>>>    		f2fs_down_write(&fi->i_gc_rwsem[WRITE]);
>>>>>>    		err = f2fs_map_blocks(inode, &map, F2FS_GET_BLOCK_PRECACHE);
>>>>>> -- 
>>>>>> 2.49.0
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>>>    			goto sync_out;
>>>>>>>    		case F2FS_GET_BLOCK_BMAP:
>>>>>>>    			map->m_pblk = 0;
>>>>>
>>>>>
>>>>> We have already reset m_flags (map->m_flags = 0) in f2fs_map_blocks().
>>>>
>>>> Zijie:
>>>>
>>>> Oops, that's right, thanks for correcting me.
>>>>
>>>>>
>>>>> I think that this bug is caused by we missed to reset m_flags when we
>>>>> goto next_dnode in below case：
>>>>>
>>>>> Data layout is something like this:
>>>>> dnode1:                     dnode2:
>>>>> [0]      A                  [0]    NEW_ADDR
>>>>> [1]      A+1                [1]    0x0
>>>>> ...
>>>>> [1016]   A+1016
>>>>> [1017]   B (B!=A+1017)      [1017] 0x0
>>>>>
>>>>> we map the last block(valid blkaddr) in dnode1:
>>>>> map->m_flags |= F2FS_MAP_MAPPED;
>>>>> map->m_pblk = blkaddr(valid blkaddr);
>>>>> map->m_len = 1;
>>>>> then we goto next_dnode, meet the first block in dnode2(hole), goto sync_out:
>>>>> map->m_flags & F2FS_MAP_MAPPED == true, and we make wrong blkaddr/len for extent_info.
>>>>
>>>> So, can you please add above explanation into commit message? that
>>>> should be helpful for understanding the problem more clearly.
>>>>
>>>> Please take a look at this case w/ your patch:
>>>>
>>>> mkfs.f2fs -O extra_attr,compression /dev/vdb -f
>>>> mount /dev/vdb /mnt/f2fs -o mode=lfs
>>>> cd /mnt/f2fs
>>>> f2fs_io write 1 0 1883 rand dsync testfile
>>>> f2fs_io fallocate 0 7712768 4096 testfile
>>>> f2fs_io write 1 1881 1 rand buffered testfile
>>>> xfs_io testfile -c "fsync"
>>>> cd /
>>>> umount /mnt/f2fs
>>>> mount /dev/vdb /mnt/f2fs
>>>> f2fs_io precache_extents /mnt/f2fs/testfile
>>>> umount /mnt/f2fs
>>>>
>>>>           f2fs_io-733     [010] .....    78.134136: f2fs_update_read_extent_tree_range: dev = (253,16), ino = 4, pgofs = 1882, len = 0, blkaddr = 17410, c_len = 0
>>>>
>>>> I suspect we need this?
>>>>
>>>> @@ -1784,7 +1781,8 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>>          }
>>>>
>>>>          if (flag == F2FS_GET_BLOCK_PRECACHE) {
>>>> -               if (map->m_flags & F2FS_MAP_MAPPED) {
>>>> +               if ((map->m_flags & F2FS_MAP_MAPPED) &&
>>>> +                       (map->m_len - ofs)) {
>>>>                          unsigned int ofs = start_pgofs - map->m_lblk;
>>>>
>>>>                          f2fs_update_read_extent_cache_range(&dn,
>>> 
>>> Thanks for pointing out this. Let me find a way to cover these cases and do more test.
>>> 
>>>> BTW, I find another bug, if one blkaddr is adjcent to previous extent,
>>>> but and it is valid, we need to set m_next_extent to pgofs rather than
>>>> pgofs + 1.
>>>>
>>>> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>>>> index cbf8841642c7..ac88ed68059c 100644
>>>> --- a/fs/f2fs/data.c
>>>> +++ b/fs/f2fs/data.c
>>>> @@ -1789,8 +1789,11 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>>>>                                  start_pgofs, map->m_pblk + ofs,
>>>>                                  map->m_len - ofs);
>>>>                  }
>>>> -               if (map->m_next_extent)
>>>> -                       *map->m_next_extent = pgofs + 1;
>>>> +               if (map->m_next_extent) {
>>>> +                       *map->m_next_extent = pgofs;
>>>> +                       if (!__is_valid_data_blkaddr(blkaddr))
>>>> +                               *map->m_next_extent += 1;
>>>> +               }
>>>>          }
>>>>          f2fs_put_dnode(&dn);
>>> 
>>> Maybe it can be this?
>>> if (map->m_next_extent)
>>> 	*map->m_next_extent = is_hole ? pgofs + 1 : pgofs;
>>
>>It's better, will update, thank you. :)
>>
>>Thanks,
>
>Hi Chao,
>I test some cases with this change:
>
>diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
>index 7961e0ddf..7093fdc95 100644
>--- a/fs/f2fs/data.c
>+++ b/fs/f2fs/data.c
>@@ -1777,13 +1777,13 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, int flag)
>        if (flag == F2FS_GET_BLOCK_PRECACHE) {
>                if (map->m_flags & F2FS_MAP_MAPPED) {
>                        unsigned int ofs = start_pgofs - map->m_lblk;
>-
>-                       f2fs_update_read_extent_cache_range(&dn,
>-                               start_pgofs, map->m_pblk + ofs,
>-                               map->m_len - ofs);
>+                       if (map->m_len - ofs > 0)
>+                               f2fs_update_read_extent_cache_range(&dn,
>+                                       start_pgofs, map->m_pblk + ofs,
>+                                       map->m_len - ofs);
>                }
>                if (map->m_next_extent)
>-                       *map->m_next_extent = pgofs + 1;
>+                       *map->m_next_extent = is_hole ? pgofs + 1 : pgofs;
>        }
>        f2fs_put_dnode(&dn);
> unlock_out:
>
>
>test cases:
>
>case1:
>dnode1:                     dnode2:
>[0]      A                  [0]    NEW_ADDR
>[1]      A+1                [1]    0x0
>...                         ....
>[1016]   A+1016
>[1017]   B (B!=A+1017)      [1017] 0x0
>
>case2:
>dnode1:                     dnode2:
>[0]      A                  [0]    C (C!=B+1)
>[1]      A+1                [1]    C+1
>...                         ....
>[1016]   A+1016
>[1017]   B (B!=A+1017)      [1017] 0x0
>
>case3:
>dnode1:                     dnode2:
>[0]      A                  [0]    C (C!=B+2)
>[1]      A+1                [1]    C+1
>...                         ....
>[1015]   A+1015
>[1016]   B (B!=A+1016)
>[1017]   B+1                [1017] 0x0
>
>case4:
>one blkaddr is adjcent to previous extent, and it is valid.
>
>And from the result, it seems this change can cover these
>situations correctly.
>Do we need a patch with this change?

Sorry, with this change, for case1:
The first block of dnode2 ([0]:NEW_ADDR) will be skipped.
Let me find a better way....