[f2fs-dev] [PATCH v3 0/3] f2fs: inline: fix minor bugs in f2fs_recover_inline_data

[f2fs-dev] [PATCH v3 0/3] f2fs: inline: fix minor bugs in f2fs_recover_inline_data

[Jack Qiu]

Jack Qiu (3):
  f2fs: inline: correct comment in f2fs_recover_inline_data
  f2fs: inline: remove redundant FI_DATA_EXIST set
  f2fs: inline: fix wrong inline inode stat

 fs/f2fs/inline.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

--
2.17.1



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH v3 1/3] f2fs: inline: correct comment in f2fs_recover_inline_data

[Jack Qiu]

In 3rd scene, it should remove data blocks instead of inline_data.

Signed-off-by: Jack Qiu <jack.qiu@huawei.com>
Reviewed-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/inline.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 70384e31788d..0399531efcd3 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -266,7 +266,7 @@ int f2fs_recover_inline_data(struct inode *inode, struct page *npage)
 	 * [prev.] [next] of inline_data flag
 	 *    o       o  -> recover inline_data
 	 *    o       x  -> remove inline_data, and then recover data blocks
-	 *    x       o  -> remove inline_data, and then recover inline_data
+	 *    x       o  -> remove data blocks, and then recover inline_data
 	 *    x       x  -> recover data blocks
 	 */
 	if (IS_INODE(npage))
--
2.17.1



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH v3 2/3] f2fs: inline: remove redundant FI_DATA_EXIST set

[Jack Qiu]

FI_DATA_EXIST has been set in recover_inline_flags, no need set in
f2fs_recover_inline_data again. Just remove it.

Signed-off-by: Jack Qiu <jack.qiu@huawei.com>
---
 fs/f2fs/inline.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 0399531efcd3..0a8f7eefca7d 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -286,7 +286,6 @@ int f2fs_recover_inline_data(struct inode *inode, struct page *npage)
 		memcpy(dst_addr, src_addr, MAX_INLINE_DATA(inode));

 		set_inode_flag(inode, FI_INLINE_DATA);
-		set_inode_flag(inode, FI_DATA_EXIST);

 		set_page_dirty(ipage);
 		f2fs_put_page(ipage, 1);
--
2.17.1



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [PATCH v3 3/3] f2fs: inline: fix wrong inline inode stat

[Jack Qiu]

Miss to stat inline inode in f2fs_recover_inline_data.

Signed-off-by: Jack Qiu <jack.qiu@huawei.com>
Reviewed-by: Chao Yu <yuchao0@huawei.com>
---
 fs/f2fs/inline.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
index 0a8f7eefca7d..c37747f7028c 100644
--- a/fs/f2fs/inline.c
+++ b/fs/f2fs/inline.c
@@ -297,6 +297,7 @@ int f2fs_recover_inline_data(struct inode *inode, struct page *npage)
 		if (IS_ERR(ipage))
 			return PTR_ERR(ipage);
 		f2fs_truncate_inline_inode(inode, ipage, 0);
+		stat_dec_inline_inode(inode);
 		clear_inode_flag(inode, FI_INLINE_DATA);
 		f2fs_put_page(ipage, 1);
 	} else if (ri && (ri->i_inline & F2FS_INLINE_DATA)) {
@@ -305,6 +306,7 @@ int f2fs_recover_inline_data(struct inode *inode, struct page *npage)
 		ret = f2fs_truncate_blocks(inode, 0, false);
 		if (ret)
 			return ret;
+		stat_inc_inline_inode(inode);
 		goto process_inline;
 	}
 	return 0;
--
2.17.1



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH v3 2/3] f2fs: inline: remove redundant FI_DATA_EXIST set

[Chao Yu]

On 2020/12/7 20:01, Jack Qiu wrote:
> FI_DATA_EXIST has been set in recover_inline_flags, no need set in
> f2fs_recover_inline_data again. Just remove it.
> 
> Signed-off-by: Jack Qiu <jack.qiu@huawei.com>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH v3 2/3] f2fs: inline: remove redundant FI_DATA_EXIST set

[Jaegeuk Kim]

On 12/07, Jack Qiu wrote:
> FI_DATA_EXIST has been set in recover_inline_flags, no need set in
> f2fs_recover_inline_data again. Just remove it.
> 
> Signed-off-by: Jack Qiu <jack.qiu@huawei.com>
> ---
>  fs/f2fs/inline.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
> index 0399531efcd3..0a8f7eefca7d 100644
> --- a/fs/f2fs/inline.c
> +++ b/fs/f2fs/inline.c
> @@ -286,7 +286,6 @@ int f2fs_recover_inline_data(struct inode *inode, struct page *npage)
>  		memcpy(dst_addr, src_addr, MAX_INLINE_DATA(inode));
> 
>  		set_inode_flag(inode, FI_INLINE_DATA);
> -		set_inode_flag(inode, FI_DATA_EXIST);

Wait, recover_inline_flags() sets this based on on-disk flag, but this tries to
recover it back.

> 
>  		set_page_dirty(ipage);
>  		f2fs_put_page(ipage, 1);
> --
> 2.17.1
> 
> 
> 
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH v3 2/3] f2fs: inline: remove redundant FI_DATA_EXIST set

[Chao Yu]

On 2020/12/8 0:49, Jaegeuk Kim wrote:
> On 12/07, Jack Qiu wrote:
>> FI_DATA_EXIST has been set in recover_inline_flags, no need set in
>> f2fs_recover_inline_data again. Just remove it.
>>
>> Signed-off-by: Jack Qiu <jack.qiu@huawei.com>
>> ---
>>   fs/f2fs/inline.c | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
>> index 0399531efcd3..0a8f7eefca7d 100644
>> --- a/fs/f2fs/inline.c
>> +++ b/fs/f2fs/inline.c
>> @@ -286,7 +286,6 @@ int f2fs_recover_inline_data(struct inode *inode, struct page *npage)
>>   		memcpy(dst_addr, src_addr, MAX_INLINE_DATA(inode));
>>
>>   		set_inode_flag(inode, FI_INLINE_DATA);
>> -		set_inode_flag(inode, FI_DATA_EXIST);
> 
> Wait, recover_inline_flags() sets this based on on-disk flag, but this tries to
> recover it back.

Should this flag only be recovered with the way like __recover_inline_status()?
otherwise, the data_exist status may be not be consistent with real condition.

Thanks,

> 
>>
>>   		set_page_dirty(ipage);
>>   		f2fs_put_page(ipage, 1);
>> --
>> 2.17.1
>>
>>
>>
>> _______________________________________________
>> Linux-f2fs-devel mailing list
>> Linux-f2fs-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> 
> 
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> .
> 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH v3 2/3] f2fs: inline: remove redundant FI_DATA_EXIST set

[Jaegeuk Kim]

On 12/08, Chao Yu wrote:
> On 2020/12/8 0:49, Jaegeuk Kim wrote:
> > On 12/07, Jack Qiu wrote:
> > > FI_DATA_EXIST has been set in recover_inline_flags, no need set in
> > > f2fs_recover_inline_data again. Just remove it.
> > > 
> > > Signed-off-by: Jack Qiu <jack.qiu@huawei.com>
> > > ---
> > >   fs/f2fs/inline.c | 1 -
> > >   1 file changed, 1 deletion(-)
> > > 
> > > diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
> > > index 0399531efcd3..0a8f7eefca7d 100644
> > > --- a/fs/f2fs/inline.c
> > > +++ b/fs/f2fs/inline.c
> > > @@ -286,7 +286,6 @@ int f2fs_recover_inline_data(struct inode *inode, struct page *npage)
> > >   		memcpy(dst_addr, src_addr, MAX_INLINE_DATA(inode));
> > > 
> > >   		set_inode_flag(inode, FI_INLINE_DATA);
> > > -		set_inode_flag(inode, FI_DATA_EXIST);
> > 
> > Wait, recover_inline_flags() sets this based on on-disk flag, but this tries to
> > recover it back.
> 
> Should this flag only be recovered with the way like __recover_inline_status()?
> otherwise, the data_exist status may be not be consistent with real condition.

Well, we cannot say consistency on this, since user can write zero data. This
can avoid __recover_inline_state() regardless of there-in data which is zero or
not.

> 
> Thanks,
> 
> > 
> > > 
> > >   		set_page_dirty(ipage);
> > >   		f2fs_put_page(ipage, 1);
> > > --
> > > 2.17.1
> > > 
> > > 
> > > 
> > > _______________________________________________
> > > Linux-f2fs-devel mailing list
> > > Linux-f2fs-devel@lists.sourceforge.net
> > > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> > 
> > 
> > _______________________________________________
> > Linux-f2fs-devel mailing list
> > Linux-f2fs-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> > .
> > 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH v3 2/3] f2fs: inline: remove redundant FI_DATA_EXIST set

[Chao Yu]

On 2020/12/8 9:38, Jaegeuk Kim wrote:
> On 12/08, Chao Yu wrote:
>> On 2020/12/8 0:49, Jaegeuk Kim wrote:
>>> On 12/07, Jack Qiu wrote:
>>>> FI_DATA_EXIST has been set in recover_inline_flags, no need set in
>>>> f2fs_recover_inline_data again. Just remove it.
>>>>
>>>> Signed-off-by: Jack Qiu <jack.qiu@huawei.com>
>>>> ---
>>>>    fs/f2fs/inline.c | 1 -
>>>>    1 file changed, 1 deletion(-)
>>>>
>>>> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
>>>> index 0399531efcd3..0a8f7eefca7d 100644
>>>> --- a/fs/f2fs/inline.c
>>>> +++ b/fs/f2fs/inline.c
>>>> @@ -286,7 +286,6 @@ int f2fs_recover_inline_data(struct inode *inode, struct page *npage)
>>>>    		memcpy(dst_addr, src_addr, MAX_INLINE_DATA(inode));
>>>>
>>>>    		set_inode_flag(inode, FI_INLINE_DATA);
>>>> -		set_inode_flag(inode, FI_DATA_EXIST);
>>>
>>> Wait, recover_inline_flags() sets this based on on-disk flag, but this tries to
>>> recover it back.
>>
>> Should this flag only be recovered with the way like __recover_inline_status()?
>> otherwise, the data_exist status may be not be consistent with real condition.
> 
> Well, we cannot say consistency on this, since user can write zero data. This

I can see that FI_DATA_EXIST flag only decide that whether f2fs_convert_inline_page()
will copy inline data into first page, if user write all zero data in inline area,
there is no need to do the copy, as first page contains all zero already.

So, IMO, the flag indicates more like a FI_NON_ZERO_DATA_EXIST status? that would be
consistent with our implementation in __recover_inline_status().

Thanks,

> can avoid __recover_inline_state() regardless of there-in data which is zero or
> not.
> 
>>
>> Thanks,
>>
>>>
>>>>
>>>>    		set_page_dirty(ipage);
>>>>    		f2fs_put_page(ipage, 1);
>>>> --
>>>> 2.17.1
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> Linux-f2fs-devel mailing list
>>>> Linux-f2fs-devel@lists.sourceforge.net
>>>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>>>
>>>
>>> _______________________________________________
>>> Linux-f2fs-devel mailing list
>>> Linux-f2fs-devel@lists.sourceforge.net
>>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>>> .
>>>
> .
> 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH v3 2/3] f2fs: inline: remove redundant FI_DATA_EXIST set

[Jaegeuk Kim]

On 12/08, Chao Yu wrote:
> On 2020/12/8 9:38, Jaegeuk Kim wrote:
> > On 12/08, Chao Yu wrote:
> > > On 2020/12/8 0:49, Jaegeuk Kim wrote:
> > > > On 12/07, Jack Qiu wrote:
> > > > > FI_DATA_EXIST has been set in recover_inline_flags, no need set in
> > > > > f2fs_recover_inline_data again. Just remove it.
> > > > > 
> > > > > Signed-off-by: Jack Qiu <jack.qiu@huawei.com>
> > > > > ---
> > > > >    fs/f2fs/inline.c | 1 -
> > > > >    1 file changed, 1 deletion(-)
> > > > > 
> > > > > diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
> > > > > index 0399531efcd3..0a8f7eefca7d 100644
> > > > > --- a/fs/f2fs/inline.c
> > > > > +++ b/fs/f2fs/inline.c
> > > > > @@ -286,7 +286,6 @@ int f2fs_recover_inline_data(struct inode *inode, struct page *npage)
> > > > >    		memcpy(dst_addr, src_addr, MAX_INLINE_DATA(inode));
> > > > > 
> > > > >    		set_inode_flag(inode, FI_INLINE_DATA);
> > > > > -		set_inode_flag(inode, FI_DATA_EXIST);
> > > > 
> > > > Wait, recover_inline_flags() sets this based on on-disk flag, but this tries to
> > > > recover it back.
> > > 
> > > Should this flag only be recovered with the way like __recover_inline_status()?
> > > otherwise, the data_exist status may be not be consistent with real condition.
> > 
> > Well, we cannot say consistency on this, since user can write zero data. This
> 
> I can see that FI_DATA_EXIST flag only decide that whether f2fs_convert_inline_page()
> will copy inline data into first page, if user write all zero data in inline area,
> there is no need to do the copy, as first page contains all zero already.
> 
> So, IMO, the flag indicates more like a FI_NON_ZERO_DATA_EXIST status? that would be
> consistent with our implementation in __recover_inline_status().

IIRC, original intention is FI_DATA_EXIST, and non-zero case was added to avoid
potential inconsistency case. So, it's not intened for FI_NON_ZERO_DATA_EXIST.
Still I don't see any problem on this, but  looking at the iteration overhead on
   do_read_inode() -> __recover_inlint_status().

> 
> Thanks,
> 
> > can avoid __recover_inline_state() regardless of there-in data which is zero or
> > not.
> > 
> > > 
> > > Thanks,
> > > 
> > > > 
> > > > > 
> > > > >    		set_page_dirty(ipage);
> > > > >    		f2fs_put_page(ipage, 1);
> > > > > --
> > > > > 2.17.1
> > > > > 
> > > > > 
> > > > > 
> > > > > _______________________________________________
> > > > > Linux-f2fs-devel mailing list
> > > > > Linux-f2fs-devel@lists.sourceforge.net
> > > > > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> > > > 
> > > > 
> > > > _______________________________________________
> > > > Linux-f2fs-devel mailing list
> > > > Linux-f2fs-devel@lists.sourceforge.net
> > > > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> > > > .
> > > > 
> > .
> > 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH v3 2/3] f2fs: inline: remove redundant FI_DATA_EXIST set

[Chao Yu]

On 2020/12/8 15:07, Jaegeuk Kim wrote:
> On 12/08, Chao Yu wrote:
>> On 2020/12/8 9:38, Jaegeuk Kim wrote:
>>> On 12/08, Chao Yu wrote:
>>>> On 2020/12/8 0:49, Jaegeuk Kim wrote:
>>>>> On 12/07, Jack Qiu wrote:
>>>>>> FI_DATA_EXIST has been set in recover_inline_flags, no need set in
>>>>>> f2fs_recover_inline_data again. Just remove it.
>>>>>>
>>>>>> Signed-off-by: Jack Qiu <jack.qiu@huawei.com>
>>>>>> ---
>>>>>>     fs/f2fs/inline.c | 1 -
>>>>>>     1 file changed, 1 deletion(-)
>>>>>>
>>>>>> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
>>>>>> index 0399531efcd3..0a8f7eefca7d 100644
>>>>>> --- a/fs/f2fs/inline.c
>>>>>> +++ b/fs/f2fs/inline.c
>>>>>> @@ -286,7 +286,6 @@ int f2fs_recover_inline_data(struct inode *inode, struct page *npage)
>>>>>>     		memcpy(dst_addr, src_addr, MAX_INLINE_DATA(inode));
>>>>>>
>>>>>>     		set_inode_flag(inode, FI_INLINE_DATA);
>>>>>> -		set_inode_flag(inode, FI_DATA_EXIST);
>>>>>
>>>>> Wait, recover_inline_flags() sets this based on on-disk flag, but this tries to
>>>>> recover it back.
>>>>
>>>> Should this flag only be recovered with the way like __recover_inline_status()?
>>>> otherwise, the data_exist status may be not be consistent with real condition.
>>>
>>> Well, we cannot say consistency on this, since user can write zero data. This
>>
>> I can see that FI_DATA_EXIST flag only decide that whether f2fs_convert_inline_page()
>> will copy inline data into first page, if user write all zero data in inline area,
>> there is no need to do the copy, as first page contains all zero already.
>>
>> So, IMO, the flag indicates more like a FI_NON_ZERO_DATA_EXIST status? that would be
>> consistent with our implementation in __recover_inline_status().
> 
> IIRC, original intention is FI_DATA_EXIST, and non-zero case was added to avoid
> potential inconsistency case. So, it's not intened for FI_NON_ZERO_DATA_EXIST.

Why we can encounter such inconsistency? is there any bug before adding that patch?

> Still I don't see any problem on this, but  looking at the iteration overhead on
>     do_read_inode() -> __recover_inlint_status().

Yup, it's trivial, I'm fine to keep it as it is.

Thanks,

> 
>>
>> Thanks,
>>
>>> can avoid __recover_inline_state() regardless of there-in data which is zero or
>>> not.
>>>
>>>>
>>>> Thanks,
>>>>
>>>>>
>>>>>>
>>>>>>     		set_page_dirty(ipage);
>>>>>>     		f2fs_put_page(ipage, 1);
>>>>>> --
>>>>>> 2.17.1
>>>>>>
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> Linux-f2fs-devel mailing list
>>>>>> Linux-f2fs-devel@lists.sourceforge.net
>>>>>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Linux-f2fs-devel mailing list
>>>>> Linux-f2fs-devel@lists.sourceforge.net
>>>>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>>>>> .
>>>>>
>>> .
>>>
> .
> 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH v3 2/3] f2fs: inline: remove redundant FI_DATA_EXIST set

[Jaegeuk Kim]

On 12/09, Chao Yu wrote:
> On 2020/12/8 15:07, Jaegeuk Kim wrote:
> > On 12/08, Chao Yu wrote:
> > > On 2020/12/8 9:38, Jaegeuk Kim wrote:
> > > > On 12/08, Chao Yu wrote:
> > > > > On 2020/12/8 0:49, Jaegeuk Kim wrote:
> > > > > > On 12/07, Jack Qiu wrote:
> > > > > > > FI_DATA_EXIST has been set in recover_inline_flags, no need set in
> > > > > > > f2fs_recover_inline_data again. Just remove it.
> > > > > > > 
> > > > > > > Signed-off-by: Jack Qiu <jack.qiu@huawei.com>
> > > > > > > ---
> > > > > > >     fs/f2fs/inline.c | 1 -
> > > > > > >     1 file changed, 1 deletion(-)
> > > > > > > 
> > > > > > > diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
> > > > > > > index 0399531efcd3..0a8f7eefca7d 100644
> > > > > > > --- a/fs/f2fs/inline.c
> > > > > > > +++ b/fs/f2fs/inline.c
> > > > > > > @@ -286,7 +286,6 @@ int f2fs_recover_inline_data(struct inode *inode, struct page *npage)
> > > > > > >     		memcpy(dst_addr, src_addr, MAX_INLINE_DATA(inode));
> > > > > > > 
> > > > > > >     		set_inode_flag(inode, FI_INLINE_DATA);
> > > > > > > -		set_inode_flag(inode, FI_DATA_EXIST);
> > > > > > 
> > > > > > Wait, recover_inline_flags() sets this based on on-disk flag, but this tries to
> > > > > > recover it back.
> > > > > 
> > > > > Should this flag only be recovered with the way like __recover_inline_status()?
> > > > > otherwise, the data_exist status may be not be consistent with real condition.
> > > > 
> > > > Well, we cannot say consistency on this, since user can write zero data. This
> > > 
> > > I can see that FI_DATA_EXIST flag only decide that whether f2fs_convert_inline_page()
> > > will copy inline data into first page, if user write all zero data in inline area,
> > > there is no need to do the copy, as first page contains all zero already.
> > > 
> > > So, IMO, the flag indicates more like a FI_NON_ZERO_DATA_EXIST status? that would be
> > > consistent with our implementation in __recover_inline_status().
> > 
> > IIRC, original intention is FI_DATA_EXIST, and non-zero case was added to avoid
> > potential inconsistency case. So, it's not intened for FI_NON_ZERO_DATA_EXIST.
> 
> Why we can encounter such inconsistency? is there any bug before adding that patch?

I can't remember exactly on it.

> 
> > Still I don't see any problem on this, but  looking at the iteration overhead on
> >     do_read_inode() -> __recover_inlint_status().
> 
> Yup, it's trivial, I'm fine to keep it as it is.
> 
> Thanks,
> 
> > 
> > > 
> > > Thanks,
> > > 
> > > > can avoid __recover_inline_state() regardless of there-in data which is zero or
> > > > not.
> > > > 
> > > > > 
> > > > > Thanks,
> > > > > 
> > > > > > 
> > > > > > > 
> > > > > > >     		set_page_dirty(ipage);
> > > > > > >     		f2fs_put_page(ipage, 1);
> > > > > > > --
> > > > > > > 2.17.1
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > _______________________________________________
> > > > > > > Linux-f2fs-devel mailing list
> > > > > > > Linux-f2fs-devel@lists.sourceforge.net
> > > > > > > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> > > > > > 
> > > > > > 
> > > > > > _______________________________________________
> > > > > > Linux-f2fs-devel mailing list
> > > > > > Linux-f2fs-devel@lists.sourceforge.net
> > > > > > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
> > > > > > .
> > > > > > 
> > > > .
> > > > 
> > .
> > 


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel