* [PATCH v4] f2fs: fix to data block override node segment by mistake
@ 2019-03-04 1:32 zhengliang
2019-03-04 4:48 ` Sahitya Tummala
0 siblings, 1 reply; 3+ messages in thread
From: zhengliang @ 2019-03-04 1:32 UTC (permalink / raw)
To: jaegeuk, yuchao0, linux-f2fs-devel
v4: Rearrange the previous three versions.
The following scenario could lead to data block override by mistake.
TASK A | TASK kworker | TASK B | TASK C
| | |
open | | |
write | | |
close | | |
| f2fs_write_data_pages | |
| f2fs_write_cache_pages | |
| f2fs_outplace_write_data | |
| f2fs_allocate_data_block (get block in seg S, | |
| S is full, and only | |
| have this valid data | |
| block) | |
| allocate_segment | |
| locate_dirty_segment (mark S as PRE) | |
| f2fs_submit_page_write (submit but is not | |
| written on dev) | |
unlink | | |
iput_final | | |
f2fs_drop_inode | | |
f2fs_truncate | | |
(not evict) | | |
| | write_checkpoint |
| | flush merged bio but not wait file data writeback |
| | set_prefree_as_free (mark S as FREE) |
| | | update NODE/DATA
| | | allocate_segment (select S)
| writeback done | |
So we need to guarantee io complete before truncate inode in f2fs_drop_inode.
Reviewed-by: Chao Yu <yuchao0@huawei.com>
Signed-off-by: Zheng Liang <zhengliang6@huawei.com>
--------
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index c46a1d43..60f0599 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -915,6 +915,10 @@ static int f2fs_drop_inode(struct inode *inode)
sb_start_intwrite(inode->i_sb);
f2fs_i_size_write(inode, 0);
+ f2fs_submit_merged_write_cond(F2FS_I_SB(inode),
+ inode, NULL, 0, DATA);
+ truncate_inode_pages_final(inode->i_mapping);
+
if (F2FS_HAS_BLOCKS(inode))
f2fs_truncate(inode);
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v4] f2fs: fix to data block override node segment by mistake
2019-03-04 1:32 [PATCH v4] f2fs: fix to data block override node segment by mistake zhengliang
@ 2019-03-04 4:48 ` Sahitya Tummala
2019-03-04 12:32 ` zhengliang
0 siblings, 1 reply; 3+ messages in thread
From: Sahitya Tummala @ 2019-03-04 4:48 UTC (permalink / raw)
To: zhengliang; +Cc: jaegeuk, linux-f2fs-devel
On Mon, Mar 04, 2019 at 09:32:25AM +0800, zhengliang wrote:
> v4: Rearrange the previous three versions.
>
> The following scenario could lead to data block override by mistake.
>
> TASK A | TASK kworker | TASK B | TASK C
> | | |
> open | | |
> write | | |
> close | | |
> | f2fs_write_data_pages | |
> | f2fs_write_cache_pages | |
> | f2fs_outplace_write_data | |
> | f2fs_allocate_data_block (get block in seg S, | |
> | S is full, and only | |
> | have this valid data | |
> | block) | |
> | allocate_segment | |
> | locate_dirty_segment (mark S as PRE) | |
In Task kworker context, if there is one valid data block allocated from S, then how can
S be marked as PRE? Shouldn't it be DIRTY? Please clarify.
> | f2fs_submit_page_write (submit but is not | |
> | written on dev) | |
> unlink | | |
> iput_final | | |
> f2fs_drop_inode | | |
> f2fs_truncate | | |
> (not evict) | | |
> | | write_checkpoint |
> | | flush merged bio but not wait file data writeback |
> | | set_prefree_as_free (mark S as FREE) |
> | | | update NODE/DATA
> | | | allocate_segment (select S)
> | writeback done | |
>
> So we need to guarantee io complete before truncate inode in f2fs_drop_inode.
>
> Reviewed-by: Chao Yu <yuchao0@huawei.com>
> Signed-off-by: Zheng Liang <zhengliang6@huawei.com>
> --------
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index c46a1d43..60f0599 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -915,6 +915,10 @@ static int f2fs_drop_inode(struct inode *inode)
> sb_start_intwrite(inode->i_sb);
> f2fs_i_size_write(inode, 0);
>
> + f2fs_submit_merged_write_cond(F2FS_I_SB(inode),
> + inode, NULL, 0, DATA);
> + truncate_inode_pages_final(inode->i_mapping);
> +
> if (F2FS_HAS_BLOCKS(inode))
> f2fs_truncate(inode);
>
>
>
> _______________________________________________
> Linux-f2fs-devel mailing list
> Linux-f2fs-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
--
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v4] f2fs: fix to data block override node segment by mistake
2019-03-04 4:48 ` Sahitya Tummala
@ 2019-03-04 12:32 ` zhengliang
0 siblings, 0 replies; 3+ messages in thread
From: zhengliang @ 2019-03-04 12:32 UTC (permalink / raw)
To: Sahitya Tummala; +Cc: jaegeuk, linux-f2fs-devel
In the case of out-of-place updating, the one valid data block will be invalid in seg S,
so S can be marked as PRE.
在 2019/3/4 12:48, Sahitya Tummala 写道:
> On Mon, Mar 04, 2019 at 09:32:25AM +0800, zhengliang wrote:
>> v4: Rearrange the previous three versions.
>>
>> The following scenario could lead to data block override by mistake.
>>
>> TASK A | TASK kworker | TASK B | TASK C
>> | | |
>> open | | |
>> write | | |
>> close | | |
>> | f2fs_write_data_pages | |
>> | f2fs_write_cache_pages | |
>> | f2fs_outplace_write_data | |
>> | f2fs_allocate_data_block (get block in seg S, | |
>> | S is full, and only | |
>> | have this valid data | |
>> | block) | |
>> | allocate_segment | |
>> | locate_dirty_segment (mark S as PRE) | |
>
> In Task kworker context, if there is one valid data block allocated from S, then how can
> S be marked as PRE? Shouldn't it be DIRTY? Please clarify.
>
>> | f2fs_submit_page_write (submit but is not | |
>> | written on dev) | |
>> unlink | | |
>> iput_final | | |
>> f2fs_drop_inode | | |
>> f2fs_truncate | | |
>> (not evict) | | |
>> | | write_checkpoint |
>> | | flush merged bio but not wait file data writeback |
>> | | set_prefree_as_free (mark S as FREE) |
>> | | | update NODE/DATA
>> | | | allocate_segment (select S)
>> | writeback done | |
>>
>> So we need to guarantee io complete before truncate inode in f2fs_drop_inode.
>>
>> Reviewed-by: Chao Yu <yuchao0@huawei.com>
>> Signed-off-by: Zheng Liang <zhengliang6@huawei.com>
>> --------
>> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
>> index c46a1d43..60f0599 100644
>> --- a/fs/f2fs/super.c
>> +++ b/fs/f2fs/super.c
>> @@ -915,6 +915,10 @@ static int f2fs_drop_inode(struct inode *inode)
>> sb_start_intwrite(inode->i_sb);
>> f2fs_i_size_write(inode, 0);
>>
>> + f2fs_submit_merged_write_cond(F2FS_I_SB(inode),
>> + inode, NULL, 0, DATA);
>> + truncate_inode_pages_final(inode->i_mapping);
>> +
>> if (F2FS_HAS_BLOCKS(inode))
>> f2fs_truncate(inode);
>>
>>
>>
>> _______________________________________________
>> 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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-03-04 12:33 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-03-04 1:32 [PATCH v4] f2fs: fix to data block override node segment by mistake zhengliang
2019-03-04 4:48 ` Sahitya Tummala
2019-03-04 12:32 ` zhengliang
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).