* [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 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-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-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: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
* 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
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).