On 2019/9/1 15:25, Jaegeuk Kim wrote: > On 08/31, Chao Yu wrote: >> On 2019/8/30 23:34, Jaegeuk Kim wrote: >>> This can guarantee inline_data has smaller i_size. >> >> So I guess "f2fs: fix to avoid corruption during inline conversion" didn't >> fix >> such corruption right, I guess checkpoint & SPO before i_size recovery will >> cause this issue? >> >> err = f2fs_convert_inline_inode(inode); >> if (err) { >> >> --> > > Yup, I think so.
Oh, we'd better to avoid wrong fixing patch as much as possible, so what about letting me change that patch to just fix error path of f2fs_convert_inline_page() by adding missing f2fs_truncate_data_blocks_range()? Meanwhile I need to add below 'Fixes' tag line: 7735730d39d7 ("f2fs: fix to propagate error from __get_meta_page()") And if possible, could you add: In below call path, if we failed to convert inline inode, inline inode may have wrong i_size which is larger than max inline size. - f2fs_setattr - truncate_setsize - f2fs_convert_inline_inode Fixes: 0cab80ee0c9e ("f2fs: fix to convert inline inode in ->setattr") > >> >> /* recover old i_size */ >> i_size_write(inode, old_size); >> return err; >> >>> >>> Signed-off-by: Jaegeuk Kim <jaeg...@kernel.org> >> >> Reviewed-by: Chao Yu <yuch...@huawei.com> >> >>> --- >>> fs/f2fs/file.c | 25 +++++++++---------------- >>> 1 file changed, 9 insertions(+), 16 deletions(-) >>> >>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c >>> index 08caaead6f16..a43193dd27cb 100644 >>> --- a/fs/f2fs/file.c >>> +++ b/fs/f2fs/file.c >>> @@ -815,14 +815,20 @@ int f2fs_setattr(struct dentry *dentry, struct iattr >>> *attr) >>> >>> if (attr->ia_valid & ATTR_SIZE) { >>> loff_t old_size = i_size_read(inode); >>> - bool to_smaller = (attr->ia_size <= old_size); >>> + >>> + if (attr->ia_size > MAX_INLINE_DATA(inode)) { >>> + /* should convert inline inode here */ >> >> Would it be better: >> >> /* should convert inline inode here in piror to i_size_write to avoid >> inconsistent status in between inline flag and i_size */ > > Put like this. > > + /* > + * should convert inline inode before i_size_write to > + * keep smaller than inline_data size with inline > flag. > + */ Better, :) thanks, > >> >> Thanks, >> >>> + err = f2fs_convert_inline_inode(inode); >>> + if (err) >>> + return err; >>> + } >>> >>> down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]); >>> down_write(&F2FS_I(inode)->i_mmap_sem); >>> >>> truncate_setsize(inode, attr->ia_size); >>> >>> - if (to_smaller) >>> + if (attr->ia_size <= old_size) >>> err = f2fs_truncate(inode); >>> /* >>> * do not trim all blocks after i_size if target size is >>> @@ -830,24 +836,11 @@ int f2fs_setattr(struct dentry *dentry, struct iattr >>> *attr) >>> */ >>> up_write(&F2FS_I(inode)->i_mmap_sem); >>> up_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]); >>> - >>> if (err) >>> return err; >>> >>> - if (!to_smaller) { >>> - /* should convert inline inode here */ >>> - if (!f2fs_may_inline_data(inode)) { >>> - err = f2fs_convert_inline_inode(inode); >>> - if (err) { >>> - /* recover old i_size */ >>> - i_size_write(inode, old_size); >>> - return err; >>> - } >>> - } >>> - inode->i_mtime = inode->i_ctime = current_time(inode); >>> - } >>> - >>> down_write(&F2FS_I(inode)->i_sem); >>> + inode->i_mtime = inode->i_ctime = current_time(inode); >>> F2FS_I(inode)->last_disk_size = i_size_read(inode); >>> up_write(&F2FS_I(inode)->i_sem); >>> } >>> > . > _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel