* [PATCH 1/3] f2fs: fix wrong value of tracepoint parameter @ 2020-05-27 10:27 ` Chao Yu 0 siblings, 0 replies; 18+ messages in thread From: Chao Yu @ 2020-05-27 10:27 UTC (permalink / raw) To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu In f2fs_lookup(), we should set @err correctly before printing it in tracepoint. Signed-off-by: Chao Yu <yuchao0@huawei.com> --- fs/f2fs/namei.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c index 63b34a161cf4..2c2f8c36c828 100644 --- a/fs/f2fs/namei.c +++ b/fs/f2fs/namei.c @@ -504,6 +504,7 @@ static struct dentry *f2fs_lookup(struct inode *dir, struct dentry *dentry, err = PTR_ERR(page); goto out; } + err = -ENOENT; goto out_splice; } @@ -549,7 +550,7 @@ static struct dentry *f2fs_lookup(struct inode *dir, struct dentry *dentry, #endif new = d_splice_alias(inode, dentry); err = PTR_ERR_OR_ZERO(new); - trace_f2fs_lookup_end(dir, dentry, ino, err); + trace_f2fs_lookup_end(dir, dentry, ino, !new ? -ENOENT : err); return new; out_iput: iput(inode); -- 2.18.0.rc1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [f2fs-dev] [PATCH 1/3] f2fs: fix wrong value of tracepoint parameter @ 2020-05-27 10:27 ` Chao Yu 0 siblings, 0 replies; 18+ messages in thread From: Chao Yu @ 2020-05-27 10:27 UTC (permalink / raw) To: jaegeuk; +Cc: linux-kernel, linux-f2fs-devel In f2fs_lookup(), we should set @err correctly before printing it in tracepoint. Signed-off-by: Chao Yu <yuchao0@huawei.com> --- fs/f2fs/namei.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c index 63b34a161cf4..2c2f8c36c828 100644 --- a/fs/f2fs/namei.c +++ b/fs/f2fs/namei.c @@ -504,6 +504,7 @@ static struct dentry *f2fs_lookup(struct inode *dir, struct dentry *dentry, err = PTR_ERR(page); goto out; } + err = -ENOENT; goto out_splice; } @@ -549,7 +550,7 @@ static struct dentry *f2fs_lookup(struct inode *dir, struct dentry *dentry, #endif new = d_splice_alias(inode, dentry); err = PTR_ERR_OR_ZERO(new); - trace_f2fs_lookup_end(dir, dentry, ino, err); + trace_f2fs_lookup_end(dir, dentry, ino, !new ? -ENOENT : err); return new; out_iput: iput(inode); -- 2.18.0.rc1 _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/3] f2fs: remove unneeded return value of __insert_discard_tree() 2020-05-27 10:27 ` [f2fs-dev] " Chao Yu @ 2020-05-27 10:27 ` Chao Yu -1 siblings, 0 replies; 18+ messages in thread From: Chao Yu @ 2020-05-27 10:27 UTC (permalink / raw) To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu We never use return value of __insert_discard_tree(), so remove it. Signed-off-by: Chao Yu <yuchao0@huawei.com> --- fs/f2fs/segment.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c index 1c48ec866b8c..ebbadde6cbce 100644 --- a/fs/f2fs/segment.c +++ b/fs/f2fs/segment.c @@ -1221,7 +1221,7 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi, return err; } -static struct discard_cmd *__insert_discard_tree(struct f2fs_sb_info *sbi, +static void __insert_discard_tree(struct f2fs_sb_info *sbi, struct block_device *bdev, block_t lstart, block_t start, block_t len, struct rb_node **insert_p, @@ -1230,7 +1230,6 @@ static struct discard_cmd *__insert_discard_tree(struct f2fs_sb_info *sbi, struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info; struct rb_node **p; struct rb_node *parent = NULL; - struct discard_cmd *dc = NULL; bool leftmost = true; if (insert_p && insert_parent) { @@ -1242,12 +1241,8 @@ static struct discard_cmd *__insert_discard_tree(struct f2fs_sb_info *sbi, p = f2fs_lookup_rb_tree_for_insert(sbi, &dcc->root, &parent, lstart, &leftmost); do_insert: - dc = __attach_discard_cmd(sbi, bdev, lstart, start, len, parent, + __attach_discard_cmd(sbi, bdev, lstart, start, len, parent, p, leftmost); - if (!dc) - return NULL; - - return dc; } static void __relocate_discard_cmd(struct discard_cmd_control *dcc, -- 2.18.0.rc1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [f2fs-dev] [PATCH 2/3] f2fs: remove unneeded return value of __insert_discard_tree() @ 2020-05-27 10:27 ` Chao Yu 0 siblings, 0 replies; 18+ messages in thread From: Chao Yu @ 2020-05-27 10:27 UTC (permalink / raw) To: jaegeuk; +Cc: linux-kernel, linux-f2fs-devel We never use return value of __insert_discard_tree(), so remove it. Signed-off-by: Chao Yu <yuchao0@huawei.com> --- fs/f2fs/segment.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c index 1c48ec866b8c..ebbadde6cbce 100644 --- a/fs/f2fs/segment.c +++ b/fs/f2fs/segment.c @@ -1221,7 +1221,7 @@ static int __submit_discard_cmd(struct f2fs_sb_info *sbi, return err; } -static struct discard_cmd *__insert_discard_tree(struct f2fs_sb_info *sbi, +static void __insert_discard_tree(struct f2fs_sb_info *sbi, struct block_device *bdev, block_t lstart, block_t start, block_t len, struct rb_node **insert_p, @@ -1230,7 +1230,6 @@ static struct discard_cmd *__insert_discard_tree(struct f2fs_sb_info *sbi, struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info; struct rb_node **p; struct rb_node *parent = NULL; - struct discard_cmd *dc = NULL; bool leftmost = true; if (insert_p && insert_parent) { @@ -1242,12 +1241,8 @@ static struct discard_cmd *__insert_discard_tree(struct f2fs_sb_info *sbi, p = f2fs_lookup_rb_tree_for_insert(sbi, &dcc->root, &parent, lstart, &leftmost); do_insert: - dc = __attach_discard_cmd(sbi, bdev, lstart, start, len, parent, + __attach_discard_cmd(sbi, bdev, lstart, start, len, parent, p, leftmost); - if (!dc) - return NULL; - - return dc; } static void __relocate_discard_cmd(struct discard_cmd_control *dcc, -- 2.18.0.rc1 _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/3] f2fs: fix to cover meta flush with cp_lock 2020-05-27 10:27 ` [f2fs-dev] " Chao Yu @ 2020-05-27 10:27 ` Chao Yu -1 siblings, 0 replies; 18+ messages in thread From: Chao Yu @ 2020-05-27 10:27 UTC (permalink / raw) To: jaegeuk; +Cc: linux-f2fs-devel, linux-kernel, chao, Chao Yu meta inode page should be flushed under cp_lock, fix it. Signed-off-by: Chao Yu <yuchao0@huawei.com> --- fs/f2fs/file.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c index f7de2a1da528..0fcae4d90074 100644 --- a/fs/f2fs/file.c +++ b/fs/f2fs/file.c @@ -2260,7 +2260,9 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg) set_sbi_flag(sbi, SBI_IS_SHUTDOWN); break; case F2FS_GOING_DOWN_METAFLUSH: + mutex_lock(&sbi->cp_mutex); f2fs_sync_meta_pages(sbi, META, LONG_MAX, FS_META_IO); + mutex_unlock(&sbi->cp_mutex); f2fs_stop_checkpoint(sbi, false); set_sbi_flag(sbi, SBI_IS_SHUTDOWN); break; -- 2.18.0.rc1 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* [f2fs-dev] [PATCH 3/3] f2fs: fix to cover meta flush with cp_lock @ 2020-05-27 10:27 ` Chao Yu 0 siblings, 0 replies; 18+ messages in thread From: Chao Yu @ 2020-05-27 10:27 UTC (permalink / raw) To: jaegeuk; +Cc: linux-kernel, linux-f2fs-devel meta inode page should be flushed under cp_lock, fix it. Signed-off-by: Chao Yu <yuchao0@huawei.com> --- fs/f2fs/file.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c index f7de2a1da528..0fcae4d90074 100644 --- a/fs/f2fs/file.c +++ b/fs/f2fs/file.c @@ -2260,7 +2260,9 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg) set_sbi_flag(sbi, SBI_IS_SHUTDOWN); break; case F2FS_GOING_DOWN_METAFLUSH: + mutex_lock(&sbi->cp_mutex); f2fs_sync_meta_pages(sbi, META, LONG_MAX, FS_META_IO); + mutex_unlock(&sbi->cp_mutex); f2fs_stop_checkpoint(sbi, false); set_sbi_flag(sbi, SBI_IS_SHUTDOWN); break; -- 2.18.0.rc1 _______________________________________________ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 3/3] f2fs: fix to cover meta flush with cp_lock 2020-05-27 10:27 ` [f2fs-dev] " Chao Yu @ 2020-05-27 21:02 ` Jaegeuk Kim -1 siblings, 0 replies; 18+ messages in thread From: Jaegeuk Kim @ 2020-05-27 21:02 UTC (permalink / raw) To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, chao On 05/27, Chao Yu wrote: > meta inode page should be flushed under cp_lock, fix it. It doesn't matter for this case, yes? > > Signed-off-by: Chao Yu <yuchao0@huawei.com> > --- > fs/f2fs/file.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c > index f7de2a1da528..0fcae4d90074 100644 > --- a/fs/f2fs/file.c > +++ b/fs/f2fs/file.c > @@ -2260,7 +2260,9 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg) > set_sbi_flag(sbi, SBI_IS_SHUTDOWN); > break; > case F2FS_GOING_DOWN_METAFLUSH: > + mutex_lock(&sbi->cp_mutex); > f2fs_sync_meta_pages(sbi, META, LONG_MAX, FS_META_IO); > + mutex_unlock(&sbi->cp_mutex); > f2fs_stop_checkpoint(sbi, false); > set_sbi_flag(sbi, SBI_IS_SHUTDOWN); > break; > -- > 2.18.0.rc1 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [f2fs-dev] [PATCH 3/3] f2fs: fix to cover meta flush with cp_lock @ 2020-05-27 21:02 ` Jaegeuk Kim 0 siblings, 0 replies; 18+ messages in thread From: Jaegeuk Kim @ 2020-05-27 21:02 UTC (permalink / raw) To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel On 05/27, Chao Yu wrote: > meta inode page should be flushed under cp_lock, fix it. It doesn't matter for this case, yes? > > Signed-off-by: Chao Yu <yuchao0@huawei.com> > --- > fs/f2fs/file.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c > index f7de2a1da528..0fcae4d90074 100644 > --- a/fs/f2fs/file.c > +++ b/fs/f2fs/file.c > @@ -2260,7 +2260,9 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg) > set_sbi_flag(sbi, SBI_IS_SHUTDOWN); > break; > case F2FS_GOING_DOWN_METAFLUSH: > + mutex_lock(&sbi->cp_mutex); > f2fs_sync_meta_pages(sbi, META, LONG_MAX, FS_META_IO); > + mutex_unlock(&sbi->cp_mutex); > f2fs_stop_checkpoint(sbi, false); > set_sbi_flag(sbi, SBI_IS_SHUTDOWN); > break; > -- > 2.18.0.rc1 _______________________________________________ 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] 18+ messages in thread
* Re: [PATCH 3/3] f2fs: fix to cover meta flush with cp_lock 2020-05-27 21:02 ` [f2fs-dev] " Jaegeuk Kim @ 2020-05-28 1:23 ` Chao Yu -1 siblings, 0 replies; 18+ messages in thread From: Chao Yu @ 2020-05-28 1:23 UTC (permalink / raw) To: Jaegeuk Kim; +Cc: linux-f2fs-devel, linux-kernel, chao On 2020/5/28 5:02, Jaegeuk Kim wrote: > On 05/27, Chao Yu wrote: >> meta inode page should be flushed under cp_lock, fix it. > > It doesn't matter for this case, yes? It's not related to discard issue. Now, I got some progress, I can reproduce that bug occasionally. Thanks, > >> >> Signed-off-by: Chao Yu <yuchao0@huawei.com> >> --- >> fs/f2fs/file.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c >> index f7de2a1da528..0fcae4d90074 100644 >> --- a/fs/f2fs/file.c >> +++ b/fs/f2fs/file.c >> @@ -2260,7 +2260,9 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg) >> set_sbi_flag(sbi, SBI_IS_SHUTDOWN); >> break; >> case F2FS_GOING_DOWN_METAFLUSH: >> + mutex_lock(&sbi->cp_mutex); >> f2fs_sync_meta_pages(sbi, META, LONG_MAX, FS_META_IO); >> + mutex_unlock(&sbi->cp_mutex); >> f2fs_stop_checkpoint(sbi, false); >> set_sbi_flag(sbi, SBI_IS_SHUTDOWN); >> break; >> -- >> 2.18.0.rc1 > . > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [f2fs-dev] [PATCH 3/3] f2fs: fix to cover meta flush with cp_lock @ 2020-05-28 1:23 ` Chao Yu 0 siblings, 0 replies; 18+ messages in thread From: Chao Yu @ 2020-05-28 1:23 UTC (permalink / raw) To: Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel On 2020/5/28 5:02, Jaegeuk Kim wrote: > On 05/27, Chao Yu wrote: >> meta inode page should be flushed under cp_lock, fix it. > > It doesn't matter for this case, yes? It's not related to discard issue. Now, I got some progress, I can reproduce that bug occasionally. Thanks, > >> >> Signed-off-by: Chao Yu <yuchao0@huawei.com> >> --- >> fs/f2fs/file.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c >> index f7de2a1da528..0fcae4d90074 100644 >> --- a/fs/f2fs/file.c >> +++ b/fs/f2fs/file.c >> @@ -2260,7 +2260,9 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg) >> set_sbi_flag(sbi, SBI_IS_SHUTDOWN); >> break; >> case F2FS_GOING_DOWN_METAFLUSH: >> + mutex_lock(&sbi->cp_mutex); >> f2fs_sync_meta_pages(sbi, META, LONG_MAX, FS_META_IO); >> + mutex_unlock(&sbi->cp_mutex); >> f2fs_stop_checkpoint(sbi, false); >> set_sbi_flag(sbi, SBI_IS_SHUTDOWN); >> break; >> -- >> 2.18.0.rc1 > . > _______________________________________________ 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] 18+ messages in thread
* Re: [PATCH 3/3] f2fs: fix to cover meta flush with cp_lock 2020-05-28 1:23 ` [f2fs-dev] " Chao Yu @ 2020-05-28 1:26 ` Jaegeuk Kim -1 siblings, 0 replies; 18+ messages in thread From: Jaegeuk Kim @ 2020-05-28 1:26 UTC (permalink / raw) To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, chao On 05/28, Chao Yu wrote: > On 2020/5/28 5:02, Jaegeuk Kim wrote: > > On 05/27, Chao Yu wrote: > >> meta inode page should be flushed under cp_lock, fix it. > > > > It doesn't matter for this case, yes? > > It's not related to discard issue. I meant we really need this or not. :P > > Now, I got some progress, I can reproduce that bug occasionally. > > Thanks, > > > > >> > >> Signed-off-by: Chao Yu <yuchao0@huawei.com> > >> --- > >> fs/f2fs/file.c | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c > >> index f7de2a1da528..0fcae4d90074 100644 > >> --- a/fs/f2fs/file.c > >> +++ b/fs/f2fs/file.c > >> @@ -2260,7 +2260,9 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg) > >> set_sbi_flag(sbi, SBI_IS_SHUTDOWN); > >> break; > >> case F2FS_GOING_DOWN_METAFLUSH: > >> + mutex_lock(&sbi->cp_mutex); > >> f2fs_sync_meta_pages(sbi, META, LONG_MAX, FS_META_IO); > >> + mutex_unlock(&sbi->cp_mutex); > >> f2fs_stop_checkpoint(sbi, false); > >> set_sbi_flag(sbi, SBI_IS_SHUTDOWN); > >> break; > >> -- > >> 2.18.0.rc1 > > . > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [f2fs-dev] [PATCH 3/3] f2fs: fix to cover meta flush with cp_lock @ 2020-05-28 1:26 ` Jaegeuk Kim 0 siblings, 0 replies; 18+ messages in thread From: Jaegeuk Kim @ 2020-05-28 1:26 UTC (permalink / raw) To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel On 05/28, Chao Yu wrote: > On 2020/5/28 5:02, Jaegeuk Kim wrote: > > On 05/27, Chao Yu wrote: > >> meta inode page should be flushed under cp_lock, fix it. > > > > It doesn't matter for this case, yes? > > It's not related to discard issue. I meant we really need this or not. :P > > Now, I got some progress, I can reproduce that bug occasionally. > > Thanks, > > > > >> > >> Signed-off-by: Chao Yu <yuchao0@huawei.com> > >> --- > >> fs/f2fs/file.c | 2 ++ > >> 1 file changed, 2 insertions(+) > >> > >> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c > >> index f7de2a1da528..0fcae4d90074 100644 > >> --- a/fs/f2fs/file.c > >> +++ b/fs/f2fs/file.c > >> @@ -2260,7 +2260,9 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg) > >> set_sbi_flag(sbi, SBI_IS_SHUTDOWN); > >> break; > >> case F2FS_GOING_DOWN_METAFLUSH: > >> + mutex_lock(&sbi->cp_mutex); > >> f2fs_sync_meta_pages(sbi, META, LONG_MAX, FS_META_IO); > >> + mutex_unlock(&sbi->cp_mutex); > >> f2fs_stop_checkpoint(sbi, false); > >> set_sbi_flag(sbi, SBI_IS_SHUTDOWN); > >> break; > >> -- > >> 2.18.0.rc1 > > . > > _______________________________________________ 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] 18+ messages in thread
* Re: [PATCH 3/3] f2fs: fix to cover meta flush with cp_lock 2020-05-28 1:26 ` [f2fs-dev] " Jaegeuk Kim @ 2020-05-28 1:47 ` Chao Yu -1 siblings, 0 replies; 18+ messages in thread From: Chao Yu @ 2020-05-28 1:47 UTC (permalink / raw) To: Jaegeuk Kim; +Cc: linux-f2fs-devel, linux-kernel, chao On 2020/5/28 9:26, Jaegeuk Kim wrote: > On 05/28, Chao Yu wrote: >> On 2020/5/28 5:02, Jaegeuk Kim wrote: >>> On 05/27, Chao Yu wrote: >>>> meta inode page should be flushed under cp_lock, fix it. >>> >>> It doesn't matter for this case, yes? >> >> It's not related to discard issue. > > I meant we really need this or not. :P Yes, let's keep that rule: flush meta pages under cp_lock, otherwise checkpoint flush order may be broken due to race, right? as checkpoint should write 2rd cp park page after flushing all meta pages. > >> >> Now, I got some progress, I can reproduce that bug occasionally. >> >> Thanks, >> >>> >>>> >>>> Signed-off-by: Chao Yu <yuchao0@huawei.com> >>>> --- >>>> fs/f2fs/file.c | 2 ++ >>>> 1 file changed, 2 insertions(+) >>>> >>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c >>>> index f7de2a1da528..0fcae4d90074 100644 >>>> --- a/fs/f2fs/file.c >>>> +++ b/fs/f2fs/file.c >>>> @@ -2260,7 +2260,9 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg) >>>> set_sbi_flag(sbi, SBI_IS_SHUTDOWN); >>>> break; >>>> case F2FS_GOING_DOWN_METAFLUSH: >>>> + mutex_lock(&sbi->cp_mutex); >>>> f2fs_sync_meta_pages(sbi, META, LONG_MAX, FS_META_IO); >>>> + mutex_unlock(&sbi->cp_mutex); >>>> f2fs_stop_checkpoint(sbi, false); >>>> set_sbi_flag(sbi, SBI_IS_SHUTDOWN); >>>> break; >>>> -- >>>> 2.18.0.rc1 >>> . >>> > . > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [f2fs-dev] [PATCH 3/3] f2fs: fix to cover meta flush with cp_lock @ 2020-05-28 1:47 ` Chao Yu 0 siblings, 0 replies; 18+ messages in thread From: Chao Yu @ 2020-05-28 1:47 UTC (permalink / raw) To: Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel On 2020/5/28 9:26, Jaegeuk Kim wrote: > On 05/28, Chao Yu wrote: >> On 2020/5/28 5:02, Jaegeuk Kim wrote: >>> On 05/27, Chao Yu wrote: >>>> meta inode page should be flushed under cp_lock, fix it. >>> >>> It doesn't matter for this case, yes? >> >> It's not related to discard issue. > > I meant we really need this or not. :P Yes, let's keep that rule: flush meta pages under cp_lock, otherwise checkpoint flush order may be broken due to race, right? as checkpoint should write 2rd cp park page after flushing all meta pages. > >> >> Now, I got some progress, I can reproduce that bug occasionally. >> >> Thanks, >> >>> >>>> >>>> Signed-off-by: Chao Yu <yuchao0@huawei.com> >>>> --- >>>> fs/f2fs/file.c | 2 ++ >>>> 1 file changed, 2 insertions(+) >>>> >>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c >>>> index f7de2a1da528..0fcae4d90074 100644 >>>> --- a/fs/f2fs/file.c >>>> +++ b/fs/f2fs/file.c >>>> @@ -2260,7 +2260,9 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg) >>>> set_sbi_flag(sbi, SBI_IS_SHUTDOWN); >>>> break; >>>> case F2FS_GOING_DOWN_METAFLUSH: >>>> + mutex_lock(&sbi->cp_mutex); >>>> f2fs_sync_meta_pages(sbi, META, LONG_MAX, FS_META_IO); >>>> + mutex_unlock(&sbi->cp_mutex); >>>> f2fs_stop_checkpoint(sbi, false); >>>> set_sbi_flag(sbi, SBI_IS_SHUTDOWN); >>>> break; >>>> -- >>>> 2.18.0.rc1 >>> . >>> > . > _______________________________________________ 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] 18+ messages in thread
* Re: [PATCH 3/3] f2fs: fix to cover meta flush with cp_lock 2020-05-28 1:47 ` [f2fs-dev] " Chao Yu @ 2020-05-28 19:00 ` Jaegeuk Kim -1 siblings, 0 replies; 18+ messages in thread From: Jaegeuk Kim @ 2020-05-28 19:00 UTC (permalink / raw) To: Chao Yu; +Cc: linux-f2fs-devel, linux-kernel, chao On 05/28, Chao Yu wrote: > On 2020/5/28 9:26, Jaegeuk Kim wrote: > > On 05/28, Chao Yu wrote: > >> On 2020/5/28 5:02, Jaegeuk Kim wrote: > >>> On 05/27, Chao Yu wrote: > >>>> meta inode page should be flushed under cp_lock, fix it. > >>> > >>> It doesn't matter for this case, yes? > >> > >> It's not related to discard issue. > > > > I meant we really need this or not. :P > > Yes, let's keep that rule: flush meta pages under cp_lock, otherwise > checkpoint flush order may be broken due to race, right? as checkpoint > should write 2rd cp park page after flushing all meta pages. Well, this is for shutdown test, and thus we don't need to sync up here. > > > > >> > >> Now, I got some progress, I can reproduce that bug occasionally. > >> > >> Thanks, > >> > >>> > >>>> > >>>> Signed-off-by: Chao Yu <yuchao0@huawei.com> > >>>> --- > >>>> fs/f2fs/file.c | 2 ++ > >>>> 1 file changed, 2 insertions(+) > >>>> > >>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c > >>>> index f7de2a1da528..0fcae4d90074 100644 > >>>> --- a/fs/f2fs/file.c > >>>> +++ b/fs/f2fs/file.c > >>>> @@ -2260,7 +2260,9 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg) > >>>> set_sbi_flag(sbi, SBI_IS_SHUTDOWN); > >>>> break; > >>>> case F2FS_GOING_DOWN_METAFLUSH: > >>>> + mutex_lock(&sbi->cp_mutex); > >>>> f2fs_sync_meta_pages(sbi, META, LONG_MAX, FS_META_IO); > >>>> + mutex_unlock(&sbi->cp_mutex); > >>>> f2fs_stop_checkpoint(sbi, false); > >>>> set_sbi_flag(sbi, SBI_IS_SHUTDOWN); > >>>> break; > >>>> -- > >>>> 2.18.0.rc1 > >>> . > >>> > > . > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [f2fs-dev] [PATCH 3/3] f2fs: fix to cover meta flush with cp_lock @ 2020-05-28 19:00 ` Jaegeuk Kim 0 siblings, 0 replies; 18+ messages in thread From: Jaegeuk Kim @ 2020-05-28 19:00 UTC (permalink / raw) To: Chao Yu; +Cc: linux-kernel, linux-f2fs-devel On 05/28, Chao Yu wrote: > On 2020/5/28 9:26, Jaegeuk Kim wrote: > > On 05/28, Chao Yu wrote: > >> On 2020/5/28 5:02, Jaegeuk Kim wrote: > >>> On 05/27, Chao Yu wrote: > >>>> meta inode page should be flushed under cp_lock, fix it. > >>> > >>> It doesn't matter for this case, yes? > >> > >> It's not related to discard issue. > > > > I meant we really need this or not. :P > > Yes, let's keep that rule: flush meta pages under cp_lock, otherwise > checkpoint flush order may be broken due to race, right? as checkpoint > should write 2rd cp park page after flushing all meta pages. Well, this is for shutdown test, and thus we don't need to sync up here. > > > > >> > >> Now, I got some progress, I can reproduce that bug occasionally. > >> > >> Thanks, > >> > >>> > >>>> > >>>> Signed-off-by: Chao Yu <yuchao0@huawei.com> > >>>> --- > >>>> fs/f2fs/file.c | 2 ++ > >>>> 1 file changed, 2 insertions(+) > >>>> > >>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c > >>>> index f7de2a1da528..0fcae4d90074 100644 > >>>> --- a/fs/f2fs/file.c > >>>> +++ b/fs/f2fs/file.c > >>>> @@ -2260,7 +2260,9 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg) > >>>> set_sbi_flag(sbi, SBI_IS_SHUTDOWN); > >>>> break; > >>>> case F2FS_GOING_DOWN_METAFLUSH: > >>>> + mutex_lock(&sbi->cp_mutex); > >>>> f2fs_sync_meta_pages(sbi, META, LONG_MAX, FS_META_IO); > >>>> + mutex_unlock(&sbi->cp_mutex); > >>>> f2fs_stop_checkpoint(sbi, false); > >>>> set_sbi_flag(sbi, SBI_IS_SHUTDOWN); > >>>> break; > >>>> -- > >>>> 2.18.0.rc1 > >>> . > >>> > > . > > _______________________________________________ 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] 18+ messages in thread
* Re: [PATCH 3/3] f2fs: fix to cover meta flush with cp_lock 2020-05-28 19:00 ` [f2fs-dev] " Jaegeuk Kim @ 2020-05-29 6:37 ` Chao Yu -1 siblings, 0 replies; 18+ messages in thread From: Chao Yu @ 2020-05-29 6:37 UTC (permalink / raw) To: Jaegeuk Kim; +Cc: linux-f2fs-devel, linux-kernel, chao On 2020/5/29 3:00, Jaegeuk Kim wrote: > On 05/28, Chao Yu wrote: >> On 2020/5/28 9:26, Jaegeuk Kim wrote: >>> On 05/28, Chao Yu wrote: >>>> On 2020/5/28 5:02, Jaegeuk Kim wrote: >>>>> On 05/27, Chao Yu wrote: >>>>>> meta inode page should be flushed under cp_lock, fix it. >>>>> >>>>> It doesn't matter for this case, yes? >>>> >>>> It's not related to discard issue. >>> >>> I meant we really need this or not. :P >> >> Yes, let's keep that rule: flush meta pages under cp_lock, otherwise >> checkpoint flush order may be broken due to race, right? as checkpoint >> should write 2rd cp park page after flushing all meta pages. > > Well, this is for shutdown test, and thus we don't need to sync up here. I'm a little worried about race condition: f2fs_write_checkpoint do_checkpoint ... shutdown f2fs_sync_meta_pages stop_checkpoint ... Though, I haven't figure out potential damage of their racing. > >> >>> >>>> >>>> Now, I got some progress, I can reproduce that bug occasionally. >>>> >>>> Thanks, >>>> >>>>> >>>>>> >>>>>> Signed-off-by: Chao Yu <yuchao0@huawei.com> >>>>>> --- >>>>>> fs/f2fs/file.c | 2 ++ >>>>>> 1 file changed, 2 insertions(+) >>>>>> >>>>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c >>>>>> index f7de2a1da528..0fcae4d90074 100644 >>>>>> --- a/fs/f2fs/file.c >>>>>> +++ b/fs/f2fs/file.c >>>>>> @@ -2260,7 +2260,9 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg) >>>>>> set_sbi_flag(sbi, SBI_IS_SHUTDOWN); >>>>>> break; >>>>>> case F2FS_GOING_DOWN_METAFLUSH: >>>>>> + mutex_lock(&sbi->cp_mutex); >>>>>> f2fs_sync_meta_pages(sbi, META, LONG_MAX, FS_META_IO); >>>>>> + mutex_unlock(&sbi->cp_mutex); >>>>>> f2fs_stop_checkpoint(sbi, false); >>>>>> set_sbi_flag(sbi, SBI_IS_SHUTDOWN); >>>>>> break; >>>>>> -- >>>>>> 2.18.0.rc1 >>>>> . >>>>> >>> . >>> > . > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [f2fs-dev] [PATCH 3/3] f2fs: fix to cover meta flush with cp_lock @ 2020-05-29 6:37 ` Chao Yu 0 siblings, 0 replies; 18+ messages in thread From: Chao Yu @ 2020-05-29 6:37 UTC (permalink / raw) To: Jaegeuk Kim; +Cc: linux-kernel, linux-f2fs-devel On 2020/5/29 3:00, Jaegeuk Kim wrote: > On 05/28, Chao Yu wrote: >> On 2020/5/28 9:26, Jaegeuk Kim wrote: >>> On 05/28, Chao Yu wrote: >>>> On 2020/5/28 5:02, Jaegeuk Kim wrote: >>>>> On 05/27, Chao Yu wrote: >>>>>> meta inode page should be flushed under cp_lock, fix it. >>>>> >>>>> It doesn't matter for this case, yes? >>>> >>>> It's not related to discard issue. >>> >>> I meant we really need this or not. :P >> >> Yes, let's keep that rule: flush meta pages under cp_lock, otherwise >> checkpoint flush order may be broken due to race, right? as checkpoint >> should write 2rd cp park page after flushing all meta pages. > > Well, this is for shutdown test, and thus we don't need to sync up here. I'm a little worried about race condition: f2fs_write_checkpoint do_checkpoint ... shutdown f2fs_sync_meta_pages stop_checkpoint ... Though, I haven't figure out potential damage of their racing. > >> >>> >>>> >>>> Now, I got some progress, I can reproduce that bug occasionally. >>>> >>>> Thanks, >>>> >>>>> >>>>>> >>>>>> Signed-off-by: Chao Yu <yuchao0@huawei.com> >>>>>> --- >>>>>> fs/f2fs/file.c | 2 ++ >>>>>> 1 file changed, 2 insertions(+) >>>>>> >>>>>> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c >>>>>> index f7de2a1da528..0fcae4d90074 100644 >>>>>> --- a/fs/f2fs/file.c >>>>>> +++ b/fs/f2fs/file.c >>>>>> @@ -2260,7 +2260,9 @@ static int f2fs_ioc_shutdown(struct file *filp, unsigned long arg) >>>>>> set_sbi_flag(sbi, SBI_IS_SHUTDOWN); >>>>>> break; >>>>>> case F2FS_GOING_DOWN_METAFLUSH: >>>>>> + mutex_lock(&sbi->cp_mutex); >>>>>> f2fs_sync_meta_pages(sbi, META, LONG_MAX, FS_META_IO); >>>>>> + mutex_unlock(&sbi->cp_mutex); >>>>>> f2fs_stop_checkpoint(sbi, false); >>>>>> set_sbi_flag(sbi, SBI_IS_SHUTDOWN); >>>>>> break; >>>>>> -- >>>>>> 2.18.0.rc1 >>>>> . >>>>> >>> . >>> > . > _______________________________________________ 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] 18+ messages in thread
end of thread, other threads:[~2020-05-29 6:37 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-27 10:27 [PATCH 1/3] f2fs: fix wrong value of tracepoint parameter Chao Yu 2020-05-27 10:27 ` [f2fs-dev] " Chao Yu 2020-05-27 10:27 ` [PATCH 2/3] f2fs: remove unneeded return value of __insert_discard_tree() Chao Yu 2020-05-27 10:27 ` [f2fs-dev] " Chao Yu 2020-05-27 10:27 ` [PATCH 3/3] f2fs: fix to cover meta flush with cp_lock Chao Yu 2020-05-27 10:27 ` [f2fs-dev] " Chao Yu 2020-05-27 21:02 ` Jaegeuk Kim 2020-05-27 21:02 ` [f2fs-dev] " Jaegeuk Kim 2020-05-28 1:23 ` Chao Yu 2020-05-28 1:23 ` [f2fs-dev] " Chao Yu 2020-05-28 1:26 ` Jaegeuk Kim 2020-05-28 1:26 ` [f2fs-dev] " Jaegeuk Kim 2020-05-28 1:47 ` Chao Yu 2020-05-28 1:47 ` [f2fs-dev] " Chao Yu 2020-05-28 19:00 ` Jaegeuk Kim 2020-05-28 19:00 ` [f2fs-dev] " Jaegeuk Kim 2020-05-29 6:37 ` Chao Yu 2020-05-29 6:37 ` [f2fs-dev] " Chao Yu
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.