* [f2fs-dev] [PATCH 1/3] f2fs: fix wrong value of tracepoint parameter
@ 2020-05-27 10:27 Chao Yu
2020-05-27 10:27 ` [f2fs-dev] [PATCH 2/3] f2fs: remove unneeded return value of __insert_discard_tree() Chao Yu
2020-05-27 10:27 ` [f2fs-dev] [PATCH 3/3] f2fs: fix to cover meta flush with cp_lock Chao Yu
0 siblings, 2 replies; 9+ 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] 9+ messages in thread
* [f2fs-dev] [PATCH 2/3] f2fs: remove unneeded return value of __insert_discard_tree()
2020-05-27 10:27 [f2fs-dev] [PATCH 1/3] f2fs: fix wrong value of tracepoint parameter Chao Yu
@ 2020-05-27 10:27 ` Chao Yu
2020-05-27 10:27 ` [f2fs-dev] [PATCH 3/3] f2fs: fix to cover meta flush with cp_lock Chao Yu
1 sibling, 0 replies; 9+ 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] 9+ messages in thread
* [f2fs-dev] [PATCH 3/3] f2fs: fix to cover meta flush with cp_lock
2020-05-27 10:27 [f2fs-dev] [PATCH 1/3] f2fs: fix wrong value of tracepoint parameter Chao Yu
2020-05-27 10:27 ` [f2fs-dev] [PATCH 2/3] f2fs: remove unneeded return value of __insert_discard_tree() Chao Yu
@ 2020-05-27 10:27 ` Chao Yu
2020-05-27 21:02 ` Jaegeuk Kim
1 sibling, 1 reply; 9+ 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] 9+ messages in thread
* Re: [f2fs-dev] [PATCH 3/3] f2fs: fix to cover meta flush with cp_lock
2020-05-27 10:27 ` [f2fs-dev] [PATCH 3/3] f2fs: fix to cover meta flush with cp_lock Chao Yu
@ 2020-05-27 21:02 ` Jaegeuk Kim
2020-05-28 1:23 ` Chao Yu
0 siblings, 1 reply; 9+ 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] 9+ 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
@ 2020-05-28 1:23 ` Chao Yu
2020-05-28 1:26 ` Jaegeuk Kim
0 siblings, 1 reply; 9+ 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] 9+ 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
@ 2020-05-28 1:26 ` Jaegeuk Kim
2020-05-28 1:47 ` Chao Yu
0 siblings, 1 reply; 9+ 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] 9+ 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
@ 2020-05-28 1:47 ` Chao Yu
2020-05-28 19:00 ` Jaegeuk Kim
0 siblings, 1 reply; 9+ 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] 9+ 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
@ 2020-05-28 19:00 ` Jaegeuk Kim
2020-05-29 6:37 ` Chao Yu
0 siblings, 1 reply; 9+ 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] 9+ 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
@ 2020-05-29 6:37 ` Chao Yu
0 siblings, 0 replies; 9+ 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] 9+ messages in thread
end of thread, other threads:[~2020-05-29 6:37 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-27 10:27 [f2fs-dev] [PATCH 1/3] f2fs: fix wrong value of tracepoint parameter Chao Yu
2020-05-27 10:27 ` [f2fs-dev] [PATCH 2/3] f2fs: remove unneeded return value of __insert_discard_tree() Chao Yu
2020-05-27 10:27 ` [f2fs-dev] [PATCH 3/3] f2fs: fix to cover meta flush with cp_lock Chao Yu
2020-05-27 21:02 ` Jaegeuk Kim
2020-05-28 1:23 ` Chao Yu
2020-05-28 1:26 ` Jaegeuk Kim
2020-05-28 1:47 ` Chao Yu
2020-05-28 19:00 ` Jaegeuk Kim
2020-05-29 6:37 ` 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).