* [PATCH] f2fs: fix m_may_create to make OPU DIO write correctly @ 2018-11-16 19:41 Jia Zhu 2018-11-19 6:24 ` Chao Yu 2018-11-19 20:29 ` [PATCH v2] " Jia Zhu 0 siblings, 2 replies; 5+ messages in thread From: Jia Zhu @ 2018-11-16 19:41 UTC (permalink / raw) To: jaegeuk, chao, yuchao0, jaizhu.zj; +Cc: Jia Zhu, linux-f2fs-devel Commit 7bc0f46d418a ("f2fs: fix out-place-update DIO write") added a parameter map.m_may_create to replace @create for triggering OPU allocation correctly. But we find it is still in-place-update when used AndroBench to test this feature.In f2fs_map_blocks(), @create has been overwritten by the code below. So the function can not allocate new block address and directly go out. code: create = dio->op == REQ_OP_WRITE; if (dio->flags & DIO_SKIP_HOLES) { if (fs_startblk <= ((i_size_read(dio->inode) -1) >>i_blkbits)) create = 0; } This patch use @map.m_may_create to replace @create to avoid this problem. Signed-off-by: Jia Zhu <zhujia13@huawei.com> --- fs/f2fs/data.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c index 8780f3d..9b61cba 100644 --- a/fs/f2fs/data.c +++ b/fs/f2fs/data.c @@ -1034,7 +1034,7 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, pgofs = (pgoff_t)map->m_lblk; end = pgofs + maxblocks; - if (!create && f2fs_lookup_extent_cache(inode, pgofs, &ei)) { + if (!map->m_may_create && f2fs_lookup_extent_cache(inode, pgofs, &ei)) { map->m_pblk = ei.blk + pgofs - ei.fofs; map->m_len = min((pgoff_t)maxblocks, ei.fofs + ei.len - pgofs); map->m_flags = F2FS_MAP_MAPPED; @@ -1093,7 +1093,7 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, set_inode_flag(inode, FI_APPEND_WRITE); } } else { - if (create) { + if (map->m_may_create) { if (unlikely(f2fs_cp_error(sbi))) { err = -EIO; goto sync_out; -- 2.10.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] f2fs: fix m_may_create to make OPU DIO write correctly 2018-11-16 19:41 [PATCH] f2fs: fix m_may_create to make OPU DIO write correctly Jia Zhu @ 2018-11-19 6:24 ` Chao Yu 2018-11-19 20:29 ` [PATCH v2] " Jia Zhu 1 sibling, 0 replies; 5+ messages in thread From: Chao Yu @ 2018-11-19 6:24 UTC (permalink / raw) To: Jia Zhu, jaegeuk, chao, jaizhu.zj; +Cc: linux-f2fs-devel Hi Jia, On 2018/11/17 3:41, Jia Zhu wrote: > Commit 7bc0f46d418a ("f2fs: fix out-place-update DIO write") Commit 7bc0f46d418a was just in dev-test branch, so the commit id will change after being merged into linus' tree. > added a parameter map.m_may_create to replace @create for > triggering OPU allocation correctly. > > But we find it is still in-place-update when used AndroBench > to test this feature.In f2fs_map_blocks(), @create has been > overwritten by the code below. So the function can not allocate > new block address and directly go out. > code: > create = dio->op == REQ_OP_WRITE; > if (dio->flags & DIO_SKIP_HOLES) { > if (fs_startblk <= ((i_size_read(dio->inode) -1) >>i_blkbits)) > create = 0; > } For DIO_SKIP_HOLES flag's semantics, it needs local filesystem to skip allocating physical data block if we encounter hole inside filesize during DIO write. How about keeping this semantics since we are still using it in f2fs_direct_IO(). err = __blockdev_direct_IO(iocb, inode, inode->i_sb->s_bdev, iter, rw == WRITE ? get_data_block_dio_write : get_data_block_dio, NULL, f2fs_dio_submit_bio, DIO_LOCKING | DIO_SKIP_HOLES); ^^^^^^^^^^^^^^^ > > This patch use @map.m_may_create to replace @create to avoid > this problem. > > Signed-off-by: Jia Zhu <zhujia13@huawei.com> > --- > fs/f2fs/data.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c > index 8780f3d..9b61cba 100644 > --- a/fs/f2fs/data.c > +++ b/fs/f2fs/data.c > @@ -1034,7 +1034,7 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, > pgofs = (pgoff_t)map->m_lblk; > end = pgofs + maxblocks; > > - if (!create && f2fs_lookup_extent_cache(inode, pgofs, &ei)) { > + if (!map->m_may_create && f2fs_lookup_extent_cache(inode, pgofs, &ei)) { So here, how about just skip returning existed block address during aligned DIO writing in LFS-mode? if (!create && f2fs_lookup_extent_cache(inode, pgofs, &ei)) { if (test_opt(sbi, LFS) && flag == F2FS_GET_BLOCK_DIO && map->m_may_create) goto next_dnode; > map->m_pblk = ei.blk + pgofs - ei.fofs; > map->m_len = min((pgoff_t)maxblocks, ei.fofs + ei.len - pgofs); > map->m_flags = F2FS_MAP_MAPPED; > @@ -1093,7 +1093,7 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, > set_inode_flag(inode, FI_APPEND_WRITE); > } > } else { > - if (create) { > + if (map->m_may_create) { We don't need to change this condition due to DIO_SKIP_HOLES, right? Thanks, > if (unlikely(f2fs_cp_error(sbi))) { > err = -EIO; > goto sync_out; > ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] f2fs: fix m_may_create to make OPU DIO write correctly 2018-11-16 19:41 [PATCH] f2fs: fix m_may_create to make OPU DIO write correctly Jia Zhu 2018-11-19 6:24 ` Chao Yu @ 2018-11-19 20:29 ` Jia Zhu 2018-11-20 11:58 ` Chao Yu 1 sibling, 1 reply; 5+ messages in thread From: Jia Zhu @ 2018-11-19 20:29 UTC (permalink / raw) To: jaegeuk, chao, yuchao0; +Cc: jaizhu.zj, Jia Zhu, linux-f2fs-devel Previously, we added a parameter @map.m_may_create to trigger OPU allocation and call f2fs_balance_fs() correctly. But in get_more_blocks(), @create has been overwritten by below code. So the function f2fs_map_blocks() will not allocate new block address but directly go out. Meanwile,there are several functions calling f2fs_map_blocks() directly and @map.m_may_create not initialized. CODE: create = dio->op == REQ_OP_WRITE; if (dio->flags & DIO_SKIP_HOLES) { if (fs_startblk <= ((i_size_read(dio->inode) - 1) >> i_blkbits)) create = 0; } This patch fixes it. Signed-off-by: Jia Zhu <zhujia13@huawei.com> --- fs/f2fs/data.c | 5 +++++ fs/f2fs/file.c | 4 +++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c index aa8843a..7226300 100644 --- a/fs/f2fs/data.c +++ b/fs/f2fs/data.c @@ -1052,6 +1052,10 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, end = pgofs + maxblocks; if (!create && f2fs_lookup_extent_cache(inode, pgofs, &ei)) { + if (test_opt(sbi, LFS) && flag == F2FS_GET_BLOCK_DIO && + map->m_may_create) + goto next_dnode; + map->m_pblk = ei.blk + pgofs - ei.fofs; map->m_len = min((pgoff_t)maxblocks, ei.fofs + ei.len - pgofs); map->m_flags = F2FS_MAP_MAPPED; @@ -1261,6 +1265,7 @@ bool f2fs_overwrite_io(struct inode *inode, loff_t pos, size_t len) map.m_next_pgofs = NULL; map.m_next_extent = NULL; map.m_seg_type = NO_CHECK_TYPE; + map.m_may_create = false; last_lblk = F2FS_BLK_ALIGN(pos + len); while (map.m_lblk < last_lblk) { diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c index 3271830..ff82350 100644 --- a/fs/f2fs/file.c +++ b/fs/f2fs/file.c @@ -2201,7 +2201,8 @@ static int f2fs_defragment_range(struct f2fs_sb_info *sbi, { struct inode *inode = file_inode(filp); struct f2fs_map_blocks map = { .m_next_extent = NULL, - .m_seg_type = NO_CHECK_TYPE }; + .m_seg_type = NO_CHECK_TYPE , + .m_may_create = false }; struct extent_info ei = {0, 0, 0}; pgoff_t pg_start, pg_end, next_pgofs; unsigned int blk_per_seg = sbi->blocks_per_seg; @@ -2935,6 +2936,7 @@ int f2fs_precache_extents(struct inode *inode) map.m_next_pgofs = NULL; map.m_next_extent = &m_next_extent; map.m_seg_type = NO_CHECK_TYPE; + map.m_may_create = false; end = F2FS_I_SB(inode)->max_file_blocks; while (map.m_lblk < end) { -- 2.10.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] f2fs: fix m_may_create to make OPU DIO write correctly 2018-11-19 20:29 ` [PATCH v2] " Jia Zhu @ 2018-11-20 11:58 ` Chao Yu 2018-11-27 3:15 ` Chao Yu 0 siblings, 1 reply; 5+ messages in thread From: Chao Yu @ 2018-11-20 11:58 UTC (permalink / raw) To: Jia Zhu, jaegeuk, yuchao0; +Cc: jaizhu.zj, linux-f2fs-devel On 2018-11-20 4:29, Jia Zhu wrote: > Previously, we added a parameter @map.m_may_create to trigger OPU > allocation and call f2fs_balance_fs() correctly. > > But in get_more_blocks(), @create has been overwritten by below code. > So the function f2fs_map_blocks() will not allocate new block address > but directly go out. Meanwile,there are several functions calling > f2fs_map_blocks() directly and @map.m_may_create not initialized. Oh, I missed to check all f2fs_map_blocks structure referrers, sorry. > CODE: > create = dio->op == REQ_OP_WRITE; > if (dio->flags & DIO_SKIP_HOLES) { > if (fs_startblk <= ((i_size_read(dio->inode) - 1) >> > i_blkbits)) > create = 0; > } > > This patch fixes it. > > Signed-off-by: Jia Zhu <zhujia13@huawei.com>> --- It will be better to add simple change logs here to indicate how you modify your patch comparing to previous one, please keep that rule for your next patch. ;) Reviewed-by: Chao Yu <yuchao0@huawei.com> Thanks, > fs/f2fs/data.c | 5 +++++ > fs/f2fs/file.c | 4 +++- > 2 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c > index aa8843a..7226300 100644 > --- a/fs/f2fs/data.c > +++ b/fs/f2fs/data.c > @@ -1052,6 +1052,10 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, > end = pgofs + maxblocks; > > if (!create && f2fs_lookup_extent_cache(inode, pgofs, &ei)) { > + if (test_opt(sbi, LFS) && flag == F2FS_GET_BLOCK_DIO && > + map->m_may_create) > + goto next_dnode; > + > map->m_pblk = ei.blk + pgofs - ei.fofs; > map->m_len = min((pgoff_t)maxblocks, ei.fofs + ei.len - pgofs); > map->m_flags = F2FS_MAP_MAPPED; > @@ -1261,6 +1265,7 @@ bool f2fs_overwrite_io(struct inode *inode, loff_t pos, size_t len) > map.m_next_pgofs = NULL; > map.m_next_extent = NULL; > map.m_seg_type = NO_CHECK_TYPE; > + map.m_may_create = false; > last_lblk = F2FS_BLK_ALIGN(pos + len); > > while (map.m_lblk < last_lblk) { > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c > index 3271830..ff82350 100644 > --- a/fs/f2fs/file.c > +++ b/fs/f2fs/file.c > @@ -2201,7 +2201,8 @@ static int f2fs_defragment_range(struct f2fs_sb_info *sbi, > { > struct inode *inode = file_inode(filp); > struct f2fs_map_blocks map = { .m_next_extent = NULL, > - .m_seg_type = NO_CHECK_TYPE }; > + .m_seg_type = NO_CHECK_TYPE , > + .m_may_create = false }; > struct extent_info ei = {0, 0, 0}; > pgoff_t pg_start, pg_end, next_pgofs; > unsigned int blk_per_seg = sbi->blocks_per_seg; > @@ -2935,6 +2936,7 @@ int f2fs_precache_extents(struct inode *inode) > map.m_next_pgofs = NULL; > map.m_next_extent = &m_next_extent; > map.m_seg_type = NO_CHECK_TYPE; > + map.m_may_create = false; > end = F2FS_I_SB(inode)->max_file_blocks; > > while (map.m_lblk < end) { > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] f2fs: fix m_may_create to make OPU DIO write correctly 2018-11-20 11:58 ` Chao Yu @ 2018-11-27 3:15 ` Chao Yu 0 siblings, 0 replies; 5+ messages in thread From: Chao Yu @ 2018-11-27 3:15 UTC (permalink / raw) To: Chao Yu, Jia Zhu, jaegeuk; +Cc: jaizhu.zj, linux-f2fs-devel Ping, On 2018/11/20 19:58, Chao Yu wrote: > On 2018-11-20 4:29, Jia Zhu wrote: >> Previously, we added a parameter @map.m_may_create to trigger OPU >> allocation and call f2fs_balance_fs() correctly. >> >> But in get_more_blocks(), @create has been overwritten by below code. >> So the function f2fs_map_blocks() will not allocate new block address >> but directly go out. Meanwile,there are several functions calling >> f2fs_map_blocks() directly and @map.m_may_create not initialized. > > Oh, I missed to check all f2fs_map_blocks structure referrers, sorry. > >> CODE: >> create = dio->op == REQ_OP_WRITE; >> if (dio->flags & DIO_SKIP_HOLES) { >> if (fs_startblk <= ((i_size_read(dio->inode) - 1) >> >> i_blkbits)) >> create = 0; >> } >> >> This patch fixes it. >> >> Signed-off-by: Jia Zhu <zhujia13@huawei.com>> --- > > It will be better to add simple change logs here to indicate how you modify your > patch comparing to previous one, please keep that rule for your next patch. ;) > > Reviewed-by: Chao Yu <yuchao0@huawei.com> > > Thanks, > >> fs/f2fs/data.c | 5 +++++ >> fs/f2fs/file.c | 4 +++- >> 2 files changed, 8 insertions(+), 1 deletion(-) >> >> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c >> index aa8843a..7226300 100644 >> --- a/fs/f2fs/data.c >> +++ b/fs/f2fs/data.c >> @@ -1052,6 +1052,10 @@ int f2fs_map_blocks(struct inode *inode, struct f2fs_map_blocks *map, >> end = pgofs + maxblocks; >> >> if (!create && f2fs_lookup_extent_cache(inode, pgofs, &ei)) { >> + if (test_opt(sbi, LFS) && flag == F2FS_GET_BLOCK_DIO && >> + map->m_may_create) >> + goto next_dnode; >> + >> map->m_pblk = ei.blk + pgofs - ei.fofs; >> map->m_len = min((pgoff_t)maxblocks, ei.fofs + ei.len - pgofs); >> map->m_flags = F2FS_MAP_MAPPED; >> @@ -1261,6 +1265,7 @@ bool f2fs_overwrite_io(struct inode *inode, loff_t pos, size_t len) >> map.m_next_pgofs = NULL; >> map.m_next_extent = NULL; >> map.m_seg_type = NO_CHECK_TYPE; >> + map.m_may_create = false; >> last_lblk = F2FS_BLK_ALIGN(pos + len); >> >> while (map.m_lblk < last_lblk) { >> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c >> index 3271830..ff82350 100644 >> --- a/fs/f2fs/file.c >> +++ b/fs/f2fs/file.c >> @@ -2201,7 +2201,8 @@ static int f2fs_defragment_range(struct f2fs_sb_info *sbi, >> { >> struct inode *inode = file_inode(filp); >> struct f2fs_map_blocks map = { .m_next_extent = NULL, >> - .m_seg_type = NO_CHECK_TYPE }; >> + .m_seg_type = NO_CHECK_TYPE , >> + .m_may_create = false }; >> struct extent_info ei = {0, 0, 0}; >> pgoff_t pg_start, pg_end, next_pgofs; >> unsigned int blk_per_seg = sbi->blocks_per_seg; >> @@ -2935,6 +2936,7 @@ int f2fs_precache_extents(struct inode *inode) >> map.m_next_pgofs = NULL; >> map.m_next_extent = &m_next_extent; >> map.m_seg_type = NO_CHECK_TYPE; >> + map.m_may_create = false; >> end = F2FS_I_SB(inode)->max_file_blocks; >> >> while (map.m_lblk < end) { >> > > . > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-11-27 3:15 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-16 19:41 [PATCH] f2fs: fix m_may_create to make OPU DIO write correctly Jia Zhu 2018-11-19 6:24 ` Chao Yu 2018-11-19 20:29 ` [PATCH v2] " Jia Zhu 2018-11-20 11:58 ` Chao Yu 2018-11-27 3:15 ` Chao Yu
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.