2019-08-30 15:37:35

by Jaegeuk Kim

[permalink] [raw]
Subject: [PATCH] f2fs: convert inline_data in prior to i_size_write

This can guarantee inline_data has smaller i_size.

Signed-off-by: Jaegeuk Kim <[email protected]>
---
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


2019-08-31 02:17:12

by Chao Yu

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH] f2fs: convert inline_data in prior to i_size_write

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 <[email protected]>

Reviewed-by: Chao Yu <[email protected]>

> ---
> 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);
> }
>

2019-09-01 07:53:26

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH] f2fs: convert inline_data in prior to i_size_write

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 <[email protected]>
>
> Reviewed-by: Chao Yu <[email protected]>
>
> > ---
> > 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);
> > }
> >

2019-09-02 01:47:40

by Chao Yu

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH] f2fs: convert inline_data in prior to i_size_write

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 <[email protected]>
>>
>> Reviewed-by: Chao Yu <[email protected]>
>>
>>> ---
>>> 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);
>>> }
>>>
> .
>

2019-09-02 23:08:41

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH] f2fs: convert inline_data in prior to i_size_write

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 <[email protected]>
> >>
> >> Reviewed-by: Chao Yu <[email protected]>
> >>
> >>> ---
> >>> 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);
> >>> }
> >>>
> > .
> >

2019-09-02 23:37:12

by Chao Yu

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH] f2fs: convert inline_data in prior to i_size_write

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 <[email protected]>
>>>>
>>>> Reviewed-by: Chao Yu <[email protected]>
>>>>
>>>>> ---
>>>>> 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
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>

2019-09-03 02:10:51

by Chao Yu

[permalink] [raw]
Subject: Re: [f2fs-dev] [PATCH] f2fs: convert inline_data in prior to i_size_write

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 <[email protected]>
>>>>>
>>>>> Reviewed-by: Chao Yu <[email protected]>
>>>>>
>>>>>> ---
>>>>>> 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
>> [email protected]
>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>>
> .
>