* [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents
@ 2025-09-10 13:58 wangzijie
2025-09-10 13:58 ` [f2fs-dev] [PATCH 2/2] f2fs: fix infinite loop in __insert_extent_tree() wangzijie
` (2 more replies)
0 siblings, 3 replies; 15+ messages in thread
From: wangzijie @ 2025-09-10 13:58 UTC (permalink / raw)
To: jaegeuk, chao; +Cc: wangzijie, linux-kernel, feng.han, linux-f2fs-devel
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [f2fs-dev] [PATCH 2/2] f2fs: fix infinite loop in __insert_extent_tree()
2025-09-10 13:58 [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents wangzijie
@ 2025-09-10 13:58 ` wangzijie
2025-09-11 3:34 ` [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents Chao Yu via Linux-f2fs-devel
2025-09-11 8:19 ` Chao Yu via Linux-f2fs-devel
2 siblings, 0 replies; 15+ messages in thread
From: wangzijie @ 2025-09-10 13:58 UTC (permalink / raw)
To: jaegeuk, chao; +Cc: wangzijie, linux-kernel, feng.han, linux-f2fs-devel
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents
2025-09-10 13:58 [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents wangzijie
2025-09-10 13:58 ` [f2fs-dev] [PATCH 2/2] f2fs: fix infinite loop in __insert_extent_tree() wangzijie
@ 2025-09-11 3:34 ` Chao Yu via Linux-f2fs-devel
2025-09-11 6:55 ` wangzijie
2025-09-11 7:42 ` wangzijie
2025-09-11 8:19 ` Chao Yu via Linux-f2fs-devel
2 siblings, 2 replies; 15+ messages in thread
From: Chao Yu via Linux-f2fs-devel @ 2025-09-11 3:34 UTC (permalink / raw)
To: wangzijie, jaegeuk; +Cc: linux-kernel, feng.han, 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents
2025-09-11 3:34 ` [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents Chao Yu via Linux-f2fs-devel
@ 2025-09-11 6:55 ` wangzijie
2025-09-11 7:47 ` Chao Yu via Linux-f2fs-devel
2025-09-11 7:42 ` wangzijie
1 sibling, 1 reply; 15+ messages in thread
From: wangzijie @ 2025-09-11 6:55 UTC (permalink / raw)
To: linux-f2fs-devel; +Cc: jaegeuk, wangzijie1, linux-kernel, feng.han
> 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents
2025-09-11 3:34 ` [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents Chao Yu via Linux-f2fs-devel
2025-09-11 6:55 ` wangzijie
@ 2025-09-11 7:42 ` wangzijie
1 sibling, 0 replies; 15+ messages in thread
From: wangzijie @ 2025-09-11 7:42 UTC (permalink / raw)
To: linux-f2fs-devel; +Cc: jaegeuk, wangzijie1, linux-kernel, feng.han
> 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents
2025-09-11 6:55 ` wangzijie
@ 2025-09-11 7:47 ` Chao Yu via Linux-f2fs-devel
0 siblings, 0 replies; 15+ messages in thread
From: Chao Yu via Linux-f2fs-devel @ 2025-09-11 7:47 UTC (permalink / raw)
To: wangzijie, linux-f2fs-devel; +Cc: jaegeuk, linux-kernel, feng.han
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents
2025-09-10 13:58 [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents wangzijie
2025-09-10 13:58 ` [f2fs-dev] [PATCH 2/2] f2fs: fix infinite loop in __insert_extent_tree() wangzijie
2025-09-11 3:34 ` [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents Chao Yu via Linux-f2fs-devel
@ 2025-09-11 8:19 ` Chao Yu via Linux-f2fs-devel
2025-09-11 9:07 ` wangzijie
2 siblings, 1 reply; 15+ messages in thread
From: Chao Yu via Linux-f2fs-devel @ 2025-09-11 8:19 UTC (permalink / raw)
To: wangzijie, jaegeuk; +Cc: linux-kernel, feng.han, 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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents
2025-09-11 8:19 ` Chao Yu via Linux-f2fs-devel
@ 2025-09-11 9:07 ` wangzijie
2025-09-12 1:52 ` Chao Yu via Linux-f2fs-devel
0 siblings, 1 reply; 15+ messages in thread
From: wangzijie @ 2025-09-11 9:07 UTC (permalink / raw)
To: linux-f2fs-devel; +Cc: jaegeuk, wangzijie1, linux-kernel, feng.han
> 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents
2025-09-11 9:07 ` wangzijie
@ 2025-09-12 1:52 ` Chao Yu via Linux-f2fs-devel
2025-09-12 3:36 ` wangzijie
0 siblings, 1 reply; 15+ messages in thread
From: Chao Yu via Linux-f2fs-devel @ 2025-09-12 1:52 UTC (permalink / raw)
To: wangzijie, linux-f2fs-devel; +Cc: jaegeuk, linux-kernel, feng.han
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents
2025-09-12 1:52 ` Chao Yu via Linux-f2fs-devel
@ 2025-09-12 3:36 ` wangzijie
2025-09-12 3:41 ` Chao Yu via Linux-f2fs-devel
0 siblings, 1 reply; 15+ messages in thread
From: wangzijie @ 2025-09-12 3:36 UTC (permalink / raw)
To: chao; +Cc: jaegeuk, wangzijie1, linux-kernel, feng.han, 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,
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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents
2025-09-12 3:36 ` wangzijie
@ 2025-09-12 3:41 ` Chao Yu via Linux-f2fs-devel
2025-09-12 10:06 ` wangzijie
0 siblings, 1 reply; 15+ messages in thread
From: Chao Yu via Linux-f2fs-devel @ 2025-09-12 3:41 UTC (permalink / raw)
To: wangzijie; +Cc: jaegeuk, linux-kernel, feng.han, 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents
2025-09-12 3:41 ` Chao Yu via Linux-f2fs-devel
@ 2025-09-12 10:06 ` wangzijie
2025-09-12 10:38 ` Chao Yu via Linux-f2fs-devel
2025-09-12 10:39 ` wangzijie
0 siblings, 2 replies; 15+ messages in thread
From: wangzijie @ 2025-09-12 10:06 UTC (permalink / raw)
To: chao; +Cc: jaegeuk, wangzijie1, linux-kernel, feng.han, 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,
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
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents
2025-09-12 10:06 ` wangzijie
@ 2025-09-12 10:38 ` Chao Yu via Linux-f2fs-devel
2025-09-12 10:48 ` wangzijie
2025-09-12 10:39 ` wangzijie
1 sibling, 1 reply; 15+ messages in thread
From: Chao Yu via Linux-f2fs-devel @ 2025-09-12 10:38 UTC (permalink / raw)
To: wangzijie; +Cc: jaegeuk, linux-kernel, feng.han, 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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents
2025-09-12 10:06 ` wangzijie
2025-09-12 10:38 ` Chao Yu via Linux-f2fs-devel
@ 2025-09-12 10:39 ` wangzijie
1 sibling, 0 replies; 15+ messages in thread
From: wangzijie @ 2025-09-12 10:39 UTC (permalink / raw)
To: wangzijie1; +Cc: jaegeuk, linux-kernel, feng.han, 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,
>
>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
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents
2025-09-12 10:38 ` Chao Yu via Linux-f2fs-devel
@ 2025-09-12 10:48 ` wangzijie
0 siblings, 0 replies; 15+ messages in thread
From: wangzijie @ 2025-09-12 10:48 UTC (permalink / raw)
To: chao; +Cc: jaegeuk, wangzijie1, linux-kernel, feng.han, 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,
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
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2025-09-12 10:48 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-10 13:58 [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents wangzijie
2025-09-10 13:58 ` [f2fs-dev] [PATCH 2/2] f2fs: fix infinite loop in __insert_extent_tree() wangzijie
2025-09-11 3:34 ` [f2fs-dev] [PATCH 1/2] f2fs: fix wrong extent_info data for precache extents Chao Yu via Linux-f2fs-devel
2025-09-11 6:55 ` wangzijie
2025-09-11 7:47 ` Chao Yu via Linux-f2fs-devel
2025-09-11 7:42 ` wangzijie
2025-09-11 8:19 ` Chao Yu via Linux-f2fs-devel
2025-09-11 9:07 ` wangzijie
2025-09-12 1:52 ` Chao Yu via Linux-f2fs-devel
2025-09-12 3:36 ` wangzijie
2025-09-12 3:41 ` Chao Yu via Linux-f2fs-devel
2025-09-12 10:06 ` wangzijie
2025-09-12 10:38 ` Chao Yu via Linux-f2fs-devel
2025-09-12 10:48 ` wangzijie
2025-09-12 10:39 ` wangzijie
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).