* [PATCH] f2fs: convert inline_data in prior to i_size_write @ 2019-08-30 15:34 Jaegeuk Kim 2019-08-31 2:16 ` [f2fs-dev] " Chao Yu 0 siblings, 1 reply; 7+ messages in thread From: Jaegeuk Kim @ 2019-08-30 15:34 UTC (permalink / raw) To: linux-kernel, linux-f2fs-devel; +Cc: Jaegeuk Kim This can guarantee inline_data has smaller i_size. Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> --- 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 */ + 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); } -- 2.19.0.605.g01d371f741-goog ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs: convert inline_data in prior to i_size_write 2019-08-30 15:34 [PATCH] f2fs: convert inline_data in prior to i_size_write Jaegeuk Kim @ 2019-08-31 2:16 ` Chao Yu 2019-09-01 7:25 ` Jaegeuk Kim 0 siblings, 1 reply; 7+ messages in thread From: Chao Yu @ 2019-08-31 2:16 UTC (permalink / raw) To: Jaegeuk Kim, linux-kernel, linux-f2fs-devel 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) { --> /* recover old i_size */ i_size_write(inode, old_size); return err; > > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> Reviewed-by: Chao Yu <yuchao0@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 */ 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); > } > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs: convert inline_data in prior to i_size_write 2019-08-31 2:16 ` [f2fs-dev] " Chao Yu @ 2019-09-01 7:25 ` Jaegeuk Kim 2019-09-02 1:45 ` Chao Yu 0 siblings, 1 reply; 7+ messages in thread From: Jaegeuk Kim @ 2019-09-01 7:25 UTC (permalink / raw) To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel 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. > > /* recover old i_size */ > i_size_write(inode, old_size); > return err; > > > > > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> > > Reviewed-by: Chao Yu <yuchao0@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. + */ > > 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); > > } > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs: convert inline_data in prior to i_size_write 2019-09-01 7:25 ` Jaegeuk Kim @ 2019-09-02 1:45 ` Chao Yu 2019-09-02 23:07 ` Jaegeuk Kim 0 siblings, 1 reply; 7+ messages in thread From: Chao Yu @ 2019-09-02 1:45 UTC (permalink / raw) To: Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel 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 <jaegeuk@kernel.org> >> >> Reviewed-by: Chao Yu <yuchao0@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); >>> } >>> > . > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs: convert inline_data in prior to i_size_write 2019-09-02 1:45 ` Chao Yu @ 2019-09-02 23:07 ` Jaegeuk Kim 2019-09-02 23:35 ` Chao Yu 0 siblings, 1 reply; 7+ messages in thread From: Jaegeuk Kim @ 2019-09-02 23:07 UTC (permalink / raw) To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel On 09/02, Chao Yu wrote: > 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()? Could you post another patch? I'm okay to combine them. > > 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 <jaegeuk@kernel.org> > >> > >> Reviewed-by: Chao Yu <yuchao0@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); > >>> } > >>> > > . > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs: convert inline_data in prior to i_size_write 2019-09-02 23:07 ` Jaegeuk Kim @ 2019-09-02 23:35 ` Chao Yu 2019-09-03 2:09 ` Chao Yu 0 siblings, 1 reply; 7+ messages in thread From: Chao Yu @ 2019-09-02 23:35 UTC (permalink / raw) To: Jaegeuk Kim, Chao Yu; +Cc: linux-kernel, linux-f2fs-devel On 2019-9-3 7:07, Jaegeuk Kim wrote: > On 09/02, Chao Yu wrote: >> 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()? > > Could you post another patch? I'm okay to combine them. No problem, let merge them in v2. :) Thanks, > >> >> 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 <jaegeuk@kernel.org> >>>> >>>> Reviewed-by: Chao Yu <yuchao0@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 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [f2fs-dev] [PATCH] f2fs: convert inline_data in prior to i_size_write 2019-09-02 23:35 ` Chao Yu @ 2019-09-03 2:09 ` Chao Yu 0 siblings, 0 replies; 7+ messages in thread From: Chao Yu @ 2019-09-03 2:09 UTC (permalink / raw) To: Chao Yu, Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel On 2019/9/3 7:35, Chao Yu wrote: > On 2019-9-3 7:07, Jaegeuk Kim wrote: >> On 09/02, Chao Yu wrote: >>> 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()? >> >> Could you post another patch? I'm okay to combine them. > > No problem, let merge them in v2. :) Still send as two separated patches which can be easily maintained in those LTS stable kernel. :P Thanks, > > Thanks, > >> >>> >>> 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 <jaegeuk@kernel.org> >>>>> >>>>> Reviewed-by: Chao Yu <yuchao0@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 >> > . > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-09-03 2:09 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-30 15:34 [PATCH] f2fs: convert inline_data in prior to i_size_write Jaegeuk Kim 2019-08-31 2:16 ` [f2fs-dev] " Chao Yu 2019-09-01 7:25 ` Jaegeuk Kim 2019-09-02 1:45 ` Chao Yu 2019-09-02 23:07 ` Jaegeuk Kim 2019-09-02 23:35 ` Chao Yu 2019-09-03 2:09 ` Chao Yu
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).